| Message ID | 43d65409eb3290b09e1c3a21cb0dc079c5f4849c.1664801307.git.jstancek@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] lib: introduce safe_write_fully() | expand |
Hi! > In case there is a short (but otherwise successful) write(), > safe_write_fully() repeats write() and attempts to resume > with the remainder of the buffer. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > include/tst_safe_macros.h | 5 +++++ > lib/tst_safe_macros.c | 19 +++++++++++++++++++ > 2 files changed, 24 insertions(+) > > diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h > index 81c4b0844267..caee0e9cf842 100644 > --- a/include/tst_safe_macros.h > +++ b/include/tst_safe_macros.h > @@ -645,4 +645,9 @@ int safe_sysinfo(const char *file, const int lineno, struct sysinfo *info); > #define SAFE_SYSINFO(info) \ > safe_sysinfo(__FILE__, __LINE__, (info)) > > +ssize_t safe_write_fully(const char *file, const int lineno, > + int fildes, const void *buf, size_t nbyte); > +#define SAFE_WRITE_FULLY(fildes, buf, nbyte) \ > + safe_write_fully(__FILE__, __LINE__, (fildes), (buf), (nbyte)) We already have a flag for SAFE_WRITE() to fail/not-fail on partial write, what about turning that into three way switch? Something as: enum safe_write_opts { SAFE_WRITE_PARTIAL = 0, SAFE_WRITE_FULL = 1, SAFE_WRITE_RETRY = 2, }; Or maybe just rename the SAFE_WRITE_FULLY() to SAFE_WRITE_RETRY(). > #endif /* SAFE_MACROS_H__ */ > diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c > index c4cdc87e7346..e4d4ef4269a4 100644 > --- a/lib/tst_safe_macros.c > +++ b/lib/tst_safe_macros.c > @@ -591,3 +591,22 @@ void safe_cmd(const char *file, const int lineno, const char *const argv[], > tst_brk_(file, lineno, TBROK, "%s failed (%d)", argv[0], rval); > } > } > + > +ssize_t safe_write_fully(const char *file, const int lineno, > + int fildes, const void *buf, size_t nbyte) > +{ > + ssize_t rval; > + size_t len = nbyte; > + > + do { > + rval = write(fildes, buf, len); > + if (rval == -1) { > + tst_brk_(file, lineno, TBROK | TERRNO, > + "write(%d,%p,%zu) failed", fildes, buf, len); I guess that we may print potentionally confusing output here since we modify the buf and len in the loop. I guess that we should store the buf pointer at the start just for a case of this message and use the nbyte and possibly write how many bytes we have managed to write before the failure. > + } > + buf += rval; > + len -= rval; > + } while (len > 0); > + > + return nbyte; > +} > -- > 2.27.0 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
On Mon, Oct 3, 2022 at 3:14 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > In case there is a short (but otherwise successful) write(), > > safe_write_fully() repeats write() and attempts to resume > > with the remainder of the buffer. > > > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > --- > > include/tst_safe_macros.h | 5 +++++ > > lib/tst_safe_macros.c | 19 +++++++++++++++++++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h > > index 81c4b0844267..caee0e9cf842 100644 > > --- a/include/tst_safe_macros.h > > +++ b/include/tst_safe_macros.h > > @@ -645,4 +645,9 @@ int safe_sysinfo(const char *file, const int lineno, struct sysinfo *info); > > #define SAFE_SYSINFO(info) \ > > safe_sysinfo(__FILE__, __LINE__, (info)) > > > > +ssize_t safe_write_fully(const char *file, const int lineno, > > + int fildes, const void *buf, size_t nbyte); > > +#define SAFE_WRITE_FULLY(fildes, buf, nbyte) \ > > + safe_write_fully(__FILE__, __LINE__, (fildes), (buf), (nbyte)) > > We already have a flag for SAFE_WRITE() to fail/not-fail on partial > write, what about turning that into three way switch? > > Something as: > > enum safe_write_opts { > SAFE_WRITE_PARTIAL = 0, > SAFE_WRITE_FULL = 1, > SAFE_WRITE_RETRY = 2, > }; I do find those names little confusing. What do you think about: SAFE_WRITE_ANY = 0 // no strictness SAFE_WRITE_ALL = 1 // all strict SAFE_WRITE_RETRY = 2 // retry > > Or maybe just rename the SAFE_WRITE_FULLY() to SAFE_WRITE_RETRY(). > > > #endif /* SAFE_MACROS_H__ */ > > diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c > > index c4cdc87e7346..e4d4ef4269a4 100644 > > --- a/lib/tst_safe_macros.c > > +++ b/lib/tst_safe_macros.c > > @@ -591,3 +591,22 @@ void safe_cmd(const char *file, const int lineno, const char *const argv[], > > tst_brk_(file, lineno, TBROK, "%s failed (%d)", argv[0], rval); > > } > > } > > + > > +ssize_t safe_write_fully(const char *file, const int lineno, > > + int fildes, const void *buf, size_t nbyte) > > +{ > > + ssize_t rval; > > + size_t len = nbyte; > > + > > + do { > > + rval = write(fildes, buf, len); > > + if (rval == -1) { > > + tst_brk_(file, lineno, TBROK | TERRNO, > > + "write(%d,%p,%zu) failed", fildes, buf, len); > > I guess that we may print potentionally confusing output here since we > modify the buf and len in the loop. I guess that we should store the buf > pointer at the start just for a case of this message and use the nbyte > and possibly write how many bytes we have managed to write before the > failure. ack > > > + } > > + buf += rval; > > + len -= rval; > > + } while (len > 0); > > + > > + return nbyte; > > +} > > -- > > 2.27.0 > > > > > > -- > > Mailing list info: https://lists.linux.it/listinfo/ltp > > -- > Cyril Hrubis > chrubis@suse.cz >
Hi! > I do find those names little confusing. What do you think about: > > SAFE_WRITE_ANY = 0 // no strictness > SAFE_WRITE_ALL = 1 // all strict > SAFE_WRITE_RETRY = 2 // retry +1 for making them shorter and more descriptive.
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 81c4b0844267..caee0e9cf842 100644 --- a/include/tst_safe_macros.h +++ b/include/tst_safe_macros.h @@ -645,4 +645,9 @@ int safe_sysinfo(const char *file, const int lineno, struct sysinfo *info); #define SAFE_SYSINFO(info) \ safe_sysinfo(__FILE__, __LINE__, (info)) +ssize_t safe_write_fully(const char *file, const int lineno, + int fildes, const void *buf, size_t nbyte); +#define SAFE_WRITE_FULLY(fildes, buf, nbyte) \ + safe_write_fully(__FILE__, __LINE__, (fildes), (buf), (nbyte)) + #endif /* SAFE_MACROS_H__ */ diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c index c4cdc87e7346..e4d4ef4269a4 100644 --- a/lib/tst_safe_macros.c +++ b/lib/tst_safe_macros.c @@ -591,3 +591,22 @@ void safe_cmd(const char *file, const int lineno, const char *const argv[], tst_brk_(file, lineno, TBROK, "%s failed (%d)", argv[0], rval); } } + +ssize_t safe_write_fully(const char *file, const int lineno, + int fildes, const void *buf, size_t nbyte) +{ + ssize_t rval; + size_t len = nbyte; + + do { + rval = write(fildes, buf, len); + if (rval == -1) { + tst_brk_(file, lineno, TBROK | TERRNO, + "write(%d,%p,%zu) failed", fildes, buf, len); + } + buf += rval; + len -= rval; + } while (len > 0); + + return nbyte; +}
In case there is a short (but otherwise successful) write(), safe_write_fully() repeats write() and attempts to resume with the remainder of the buffer. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- include/tst_safe_macros.h | 5 +++++ lib/tst_safe_macros.c | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+)