Re: [LTP] [PATCH v1] tst_cgroup: use tmpdir

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

> Hi Edward,
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Although it's oke to have 3 lines duplicate, it'd be better to put it into
> function, because we already have few places with this code
> (_supported_fs_types.c, getcwd02.c, filecaps_common.h, openposix: tempfile.h).
>
> I'm not sure myself for a correct place, maybe some file in lib/*.c, which uses
> tst_fs.h (maybe we could merge some of these files which contain single function
> into tst_fs.c and put new function here).
>
> I'm ok to merge it as is and find correct place as another effort.

OK, pushed with added whitespace, thanks!

>
> Kind regards,
> Petr
>
>> Use tmpdir instead of hardcoded /tmp path.
>
>> Signed-off-by: Edward Liaw <edliaw@google.com>
>> ---
>>  lib/tst_cgroup.c | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>
>> diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c
>> index 6c015e4f8..458b823f2 100644
>> --- a/lib/tst_cgroup.c
>> +++ b/lib/tst_cgroup.c
>> @@ -290,7 +290,7 @@ static struct cgroup_ctrl controllers[] = {
>>  static const char *cgroup_ltp_dir = "ltp";
>>  static const char *cgroup_ltp_drain_dir = "drain";
>>  static char cgroup_test_dir[NAME_MAX + 1];
>> -static const char *cgroup_mount_ltp_prefix = "/tmp/cgroup_";
>> +static const char *cgroup_mount_ltp_prefix = "cgroup_";
>>  static const char *cgroup_v2_ltp_mount = "unified";
>
>>  #define first_root				\
>> @@ -645,8 +645,12 @@ static void cgroup_mount_v2(void)
>>  {
>>  	int ret;
>>  	char mnt_path[PATH_MAX];
>> +	const char *tmpdir = getenv("TMPDIR");
>> +	if (!tmpdir)
>> +		tmpdir = "/tmp";
>
>> -	sprintf(mnt_path, "%s%s", cgroup_mount_ltp_prefix, cgroup_v2_ltp_mount);
>> +	sprintf(mnt_path, "%s/%s%s",
>> +		tmpdir, cgroup_mount_ltp_prefix, cgroup_v2_ltp_mount);
>
>>  	if (!mkdir(mnt_path, 0777)) {
>>  		roots[0].mnt_dir.we_created_it = 1;
>> @@ -693,6 +697,9 @@ static void cgroup_mount_v1(struct cgroup_ctrl *const ctrl)
>>  {
>>  	char mnt_path[PATH_MAX];
>>  	int made_dir = 0;
>> +	const char *tmpdir = getenv("TMPDIR");
>> +	if (!tmpdir)
>> +		tmpdir = "/tmp";
>
>>  	if (ctrl->ctrl_indx == CTRL_BLKIO && controllers[CTRL_IO].ctrl_root) {
>>  		tst_res(TCONF,
>> @@ -700,7 +707,8 @@ static void cgroup_mount_v1(struct cgroup_ctrl *const ctrl)
>>  		return;
>>  	}
>
>> -	sprintf(mnt_path, "%s%s", cgroup_mount_ltp_prefix, ctrl->ctrl_name);
>> +	sprintf(mnt_path, "%s/%s%s",
>> +		tmpdir, cgroup_mount_ltp_prefix, ctrl->ctrl_name);
>
>>  	if (!mkdir(mnt_path, 0777)) {
>>  		made_dir = 1;


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp