| Message ID | 20180910141901.20541-2-cfamullaconrad@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5] setregid01: Convert to newlib | expand |
Hi! > +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid); > +#define SAFE_GETGRGID(gid) \ > + safe_getgrgid(__FILE__, __LINE__, (gid)) > + > int safe_setxattr(const char *file, const int lineno, const char *path, > const char *name, const void *value, size_t size, int flags); > #define SAFE_SETXATTR(path, name, value, size, flags) \ > diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c > index 17384f32c..2e041c460 100644 > --- a/lib/tst_safe_macros.c > +++ b/lib/tst_safe_macros.c > @@ -153,6 +153,19 @@ struct group *safe_getgrnam(const char *file, const int lineno, > return rval; > } > > +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid) > +{ > + struct group *rval; Looking at manual pages we should zero the errno here, since we print it in the tst_brk_() in a case of NULL. > + rval = getgrgid(gid); > + if (rval == NULL) { > + tst_brk_(file, lineno, TBROK | TERRNO, > + "getgrgid(%li) failed", (long)gid); > + } I also wonder if it's okay to break the test in case that the entry wasn't found (the return value would be NULL and errno would be untouched) but I guess that it's OK for most of the cases. > + return rval; > +} > + > int safe_chroot(const char *file, const int lineno, const char *path) > { > int rval; > -- > 2.16.4 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi, On 10/2/18 4:20 PM, Cyril Hrubis wrote: > Hi! >> +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid); >> +#define SAFE_GETGRGID(gid) \ >> + safe_getgrgid(__FILE__, __LINE__, (gid)) >> + >> int safe_setxattr(const char *file, const int lineno, const char *path, >> const char *name, const void *value, size_t size, int flags); >> #define SAFE_SETXATTR(path, name, value, size, flags) \ >> diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c >> index 17384f32c..2e041c460 100644 >> --- a/lib/tst_safe_macros.c >> +++ b/lib/tst_safe_macros.c >> @@ -153,6 +153,19 @@ struct group *safe_getgrnam(const char *file, const int lineno, >> return rval; >> } >> >> +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid) >> +{ >> + struct group *rval; > > Looking at manual pages we should zero the errno here, since we print it > in the tst_brk_() in a case of NULL. Thanks for pointing that. I'm going to change it here and in getgrnam(). > >> + rval = getgrgid(gid); >> + if (rval == NULL) { >> + tst_brk_(file, lineno, TBROK | TERRNO, >> + "getgrgid(%li) failed", (long)gid); >> + } > > I also wonder if it's okay to break the test in case that the entry > wasn't found (the return value would be NULL and errno would be > untouched) but I guess that it's OK for most of the cases. I think so too. Also I thought these safe_* functions are more for setting up the test, where it is more likely requesting existing groups. Maybe for the fallback approach it could be handy. But if we really need more then two options, I think passing a array would do it. Or should we go with an array now? > >> + return rval; >> +} >> + >> int safe_chroot(const char *file, const int lineno, const char *path) >> { >> int rval; >> -- >> 2.16.4 >> >> >> -- >> Mailing list info: https://lists.linux.it/listinfo/ltp >
Hi! > >> + rval = getgrgid(gid); > >> + if (rval == NULL) { > >> + tst_brk_(file, lineno, TBROK | TERRNO, > >> + "getgrgid(%li) failed", (long)gid); > >> + } > > > > I also wonder if it's okay to break the test in case that the entry > > wasn't found (the return value would be NULL and errno would be > > untouched) but I guess that it's OK for most of the cases. > > I think so too. Also I thought these safe_* functions are more for > setting up the test, where it is more likely requesting existing groups. > > Maybe for the fallback approach it could be handy. But if we really need > more then two options, I think passing a array would do it. > Or should we go with an array now? Unless there is a case for it I woudln't bother at this point.
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 03657a410..d457ae92a 100644 --- a/include/tst_safe_macros.h +++ b/include/tst_safe_macros.h @@ -441,6 +441,10 @@ struct group *safe_getgrnam(const char *file, const int lineno, #define SAFE_GETGRNAM(name) \ safe_getgrnam(__FILE__, __LINE__, (name)) +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid); +#define SAFE_GETGRGID(gid) \ + safe_getgrgid(__FILE__, __LINE__, (gid)) + int safe_setxattr(const char *file, const int lineno, const char *path, const char *name, const void *value, size_t size, int flags); #define SAFE_SETXATTR(path, name, value, size, flags) \ diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c index 17384f32c..2e041c460 100644 --- a/lib/tst_safe_macros.c +++ b/lib/tst_safe_macros.c @@ -153,6 +153,19 @@ struct group *safe_getgrnam(const char *file, const int lineno, return rval; } +struct group *safe_getgrgid(const char *file, const int lineno, gid_t gid) +{ + struct group *rval; + + rval = getgrgid(gid); + if (rval == NULL) { + tst_brk_(file, lineno, TBROK | TERRNO, + "getgrgid(%li) failed", (long)gid); + } + + return rval; +} + int safe_chroot(const char *file, const int lineno, const char *path) { int rval;
Add tst safe wrapper for getgrgid() function. Signed-off-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de> --- include/tst_safe_macros.h | 4 ++++ lib/tst_safe_macros.c | 13 +++++++++++++ 2 files changed, 17 insertions(+)