| Message ID | 20221027181531.3686476-1-edliaw@google.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1] tst_cgroup: use tmpdir | expand |
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. 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;
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;
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;
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(-)