| Message ID | 20220616140717.23708-2-andrea.cervesato@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Refactor mqns testing suite | expand |
Hi Andrea, > Added SAFE_MQ_UNLINK and SAFE_MQ_CLOSE in tst_safe_posix_ipc.h LGTM, few minor notes below. Reviewed-by: Petr Vorel <petr.vorel@gmail.com> > +++ b/include/tst_safe_posix_ipc.h > @@ -11,6 +11,10 @@ > #define SAFE_MQ_OPEN(pathname, oflags, ...) \ > safe_mq_open(__FILE__, __LINE__, (pathname), (oflags), ##__VA_ARGS__) I'd add blank line here (readability) > +#define SAFE_MQ_CLOSE(__mqdes) \ > + safe_mq_close(__FILE__, __LINE__, (__mqdes)) and here. > +#define SAFE_MQ_UNLINK(name) \ > + safe_mq_unlink(__FILE__, __LINE__, (name)) > static inline int safe_mq_open(const char *file, const int lineno, > const char *pathname, int oflags, ...) > @@ -46,4 +50,34 @@ static inline int safe_mq_open(const char *file, const int lineno, > return rval; > } > +static inline int safe_mq_close(const char *file, const int lineno, > + mqd_t __mqdes) > +{ > + int rval; > + > + rval = mq_close(__mqdes); > + > + if (rval == -1) { > + tst_brk_(file, lineno, TBROK | TERRNO, > + "mq_close(%d) failed", __mqdes); > + } How about check for invalid return value? } else if (rval < 0) { tst_brk_(file, lineno, TBROK | TERRNO, "Invalid mq_close(%d) return value %d", __mqdes, rval); } > + > + return rval; > +} > + > +static inline int safe_mq_unlink(const char *file, const int lineno, > + const char* name) > +{ > + int rval; > + > + rval = mq_unlink(name); > + > + if (rval == -1) { > + tst_brk_(file, lineno, TBROK | TERRNO, > + "mq_unlink(%s) failed", name); > + } And here. Kind regards, Petr > + > + return rval; > +} > + > #endif /* TST_SAFE_POSIX_IPC_H__ */
Hi Andrea, ... > > +static inline int safe_mq_close(const char *file, const int lineno, > > + mqd_t __mqdes) > > +{ > > + int rval; > > + > > + rval = mq_close(__mqdes); > > + > > + if (rval == -1) { > > + tst_brk_(file, lineno, TBROK | TERRNO, > > + "mq_close(%d) failed", __mqdes); > > + } > How about check for invalid return value? > } else if (rval < 0) { > tst_brk_(file, lineno, TBROK | TERRNO, > "Invalid mq_close(%d) return value %d", __mqdes, rval); > } Also safe_mq_open() could have check for invalid return value. Kind regards, Petr
Hi! On 6/16/22 23:57, Petr Vorel wrote: > Hi Andrea, > > ... >>> +static inline int safe_mq_close(const char *file, const int lineno, >>> + mqd_t __mqdes) >>> +{ >>> + int rval; >>> + >>> + rval = mq_close(__mqdes); >>> + >>> + if (rval == -1) { >>> + tst_brk_(file, lineno, TBROK | TERRNO, >>> + "mq_close(%d) failed", __mqdes); >>> + } >> How about check for invalid return value? >> } else if (rval < 0) { >> tst_brk_(file, lineno, TBROK | TERRNO, >> "Invalid mq_close(%d) return value %d", __mqdes, rval); >> } > Also safe_mq_open() could have check for invalid return value. > It's already like that, Isn't it? > Kind regards, > Petr Andrea
Hi Andrea, > Hi! > On 6/16/22 23:57, Petr Vorel wrote: > > Hi Andrea, > > ... > > > > +static inline int safe_mq_close(const char *file, const int lineno, > > > > + mqd_t __mqdes) > > > > +{ > > > > + int rval; > > > > + > > > > + rval = mq_close(__mqdes); > > > > + > > > > + if (rval == -1) { > > > > + tst_brk_(file, lineno, TBROK | TERRNO, > > > > + "mq_close(%d) failed", __mqdes); > > > > + } > > > How about check for invalid return value? > > > } else if (rval < 0) { > > > tst_brk_(file, lineno, TBROK | TERRNO, > > > "Invalid mq_close(%d) return value %d", __mqdes, rval); > > > } > > Also safe_mq_open() could have check for invalid return value. > It's already like that, Isn't it? No it's not. But looking for v3 of this patchset (v3) you have added it. Thanks! Kind regards, Petr > > Kind regards, > > Petr > Andrea
diff --git a/include/tst_safe_posix_ipc.h b/include/tst_safe_posix_ipc.h index b60c12c9e..3650209ae 100644 --- a/include/tst_safe_posix_ipc.h +++ b/include/tst_safe_posix_ipc.h @@ -11,6 +11,10 @@ #define SAFE_MQ_OPEN(pathname, oflags, ...) \ safe_mq_open(__FILE__, __LINE__, (pathname), (oflags), ##__VA_ARGS__) +#define SAFE_MQ_CLOSE(__mqdes) \ + safe_mq_close(__FILE__, __LINE__, (__mqdes)) +#define SAFE_MQ_UNLINK(name) \ + safe_mq_unlink(__FILE__, __LINE__, (name)) static inline int safe_mq_open(const char *file, const int lineno, const char *pathname, int oflags, ...) @@ -46,4 +50,34 @@ static inline int safe_mq_open(const char *file, const int lineno, return rval; } +static inline int safe_mq_close(const char *file, const int lineno, + mqd_t __mqdes) +{ + int rval; + + rval = mq_close(__mqdes); + + if (rval == -1) { + tst_brk_(file, lineno, TBROK | TERRNO, + "mq_close(%d) failed", __mqdes); + } + + return rval; +} + +static inline int safe_mq_unlink(const char *file, const int lineno, + const char* name) +{ + int rval; + + rval = mq_unlink(name); + + if (rval == -1) { + tst_brk_(file, lineno, TBROK | TERRNO, + "mq_unlink(%s) failed", name); + } + + return rval; +} + #endif /* TST_SAFE_POSIX_IPC_H__ */
Added SAFE_MQ_UNLINK and SAFE_MQ_CLOSE in tst_safe_posix_ipc.h Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> --- include/tst_safe_posix_ipc.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)