* [LTP] [PATCH 1/3] lib: introduce safe_write_fully()
@ 2022-10-03 12:48 Jan Stancek
2022-10-03 12:48 ` [LTP] [PATCH 2/3] syscalls/preadv203: don't treat short write() as failure Jan Stancek
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Jan Stancek @ 2022-10-03 12:48 UTC (permalink / raw)
To: ltp
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))
+
#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;
+}
--
2.27.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 13+ messages in thread* [LTP] [PATCH 2/3] syscalls/preadv203: don't treat short write() as failure 2022-10-03 12:48 [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Jan Stancek @ 2022-10-03 12:48 ` Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 3/3] mmapstress04: use SAFE_WRITE_FULLY from LTP library Jan Stancek ` (2 subsequent siblings) 3 siblings, 0 replies; 13+ messages in thread From: Jan Stancek @ 2022-10-03 12:48 UTC (permalink / raw) To: ltp Test is sporadically running into TBROK in setup() due to short write: tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts='' preadv203.c:257: TBROK: write(501,0x3fff4ef7d15,4123) failed: SUCCESS (0) Switch to SAFE_WRITE_FULLY(). Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/syscalls/preadv2/preadv203.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testcases/kernel/syscalls/preadv2/preadv203.c b/testcases/kernel/syscalls/preadv2/preadv203.c index 60dc4a882f16..f3e3ef438de2 100644 --- a/testcases/kernel/syscalls/preadv2/preadv203.c +++ b/testcases/kernel/syscalls/preadv2/preadv203.c @@ -254,7 +254,7 @@ static void setup(void) for (j = 0; j < CHUNKS; j++) { memset(buf, '0' + j, sizeof(buf)); - SAFE_WRITE(1, fds[i], buf, sizeof(buf)); + SAFE_WRITE_FULLY(fds[i], buf, sizeof(buf)); } } } -- 2.27.0 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [PATCH 3/3] mmapstress04: use SAFE_WRITE_FULLY from LTP library 2022-10-03 12:48 [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 2/3] syscalls/preadv203: don't treat short write() as failure Jan Stancek @ 2022-10-03 12:48 ` Jan Stancek 2022-10-03 13:16 ` [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Cyril Hrubis 2022-10-04 8:31 ` [LTP] [PATCH v2] lib: introduce safe_write() retry Jan Stancek 3 siblings, 0 replies; 13+ messages in thread From: Jan Stancek @ 2022-10-03 12:48 UTC (permalink / raw) To: ltp Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/mem/mmapstress/mmapstress04.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/testcases/kernel/mem/mmapstress/mmapstress04.c b/testcases/kernel/mem/mmapstress/mmapstress04.c index ceede7eaa860..2b421890a461 100644 --- a/testcases/kernel/mem/mmapstress/mmapstress04.c +++ b/testcases/kernel/mem/mmapstress/mmapstress04.c @@ -32,17 +32,6 @@ static void setup(void) MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); } -static void write_fully(int fd, void *buf, int len) -{ - int ret; - - do { - ret = SAFE_WRITE(0, fd, buf, len); - buf += ret; - len -= ret; - } while (len > 0); -} - static void mmapstress04(void) { int i, j, rofd, rwfd; @@ -85,7 +74,7 @@ static void mmapstress04(void) buf = SAFE_MALLOC(page_size); memset(buf, 'a', page_size); for (i = 0; i < 6 * 64; i++) - write_fully(rwfd, buf, page_size); + SAFE_WRITE_FULLY(rwfd, buf, page_size); free(buf); SAFE_CLOSE(rwfd); -- 2.27.0 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3] lib: introduce safe_write_fully() 2022-10-03 12:48 [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 2/3] syscalls/preadv203: don't treat short write() as failure Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 3/3] mmapstress04: use SAFE_WRITE_FULLY from LTP library Jan Stancek @ 2022-10-03 13:16 ` Cyril Hrubis 2022-10-03 13:56 ` Jan Stancek 2022-10-04 8:31 ` [LTP] [PATCH v2] lib: introduce safe_write() retry Jan Stancek 3 siblings, 1 reply; 13+ messages in thread From: Cyril Hrubis @ 2022-10-03 13:16 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp 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 -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3] lib: introduce safe_write_fully() 2022-10-03 13:16 ` [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Cyril Hrubis @ 2022-10-03 13:56 ` Jan Stancek 2022-10-03 14:00 ` Cyril Hrubis 0 siblings, 1 reply; 13+ messages in thread From: Jan Stancek @ 2022-10-03 13:56 UTC (permalink / raw) To: Cyril Hrubis; +Cc: 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 > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH 1/3] lib: introduce safe_write_fully() 2022-10-03 13:56 ` Jan Stancek @ 2022-10-03 14:00 ` Cyril Hrubis 0 siblings, 0 replies; 13+ messages in thread From: Cyril Hrubis @ 2022-10-03 14:00 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp 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. -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-03 12:48 [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Jan Stancek ` (2 preceding siblings ...) 2022-10-03 13:16 ` [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Cyril Hrubis @ 2022-10-04 8:31 ` Jan Stancek 2022-10-04 12:20 ` Petr Vorel 2022-10-05 9:32 ` Cyril Hrubis 3 siblings, 2 replies; 13+ messages in thread From: Jan Stancek @ 2022-10-04 8:31 UTC (permalink / raw) To: ltp Turn safe_write() len_strict parameter into 3-way switch, introducing one additional mode of operation "retry". On short writes, this resumes write() with remainder of the buffer. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- include/safe_macros_fn.h | 11 ++++++++-- lib/safe_macros.c | 42 ++++++++++++++++++++++++++++--------- lib/tests/tst_safe_macros.c | 6 +++--- 3 files changed, 44 insertions(+), 15 deletions(-) I can send follow-up to update all call sites too. diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h index 3df952811b94..8eb03edd81ba 100644 --- a/include/safe_macros_fn.h +++ b/include/safe_macros_fn.h @@ -24,6 +24,13 @@ #include <unistd.h> #include <dirent.h> +/* supported values for safe_write() len_strict parameter */ +enum safe_write_opts { + SAFE_WRITE_ANY = 0, // no length strictness, short writes are ok + SAFE_WRITE_ALL = 1, // strict length, short writes raise TBROK + SAFE_WRITE_RETRY = 2, // retry/resume after short write +}; + char* safe_basename(const char *file, const int lineno, void (*cleanup_fn)(void), char *path); @@ -111,8 +118,8 @@ int safe_symlink(const char *file, const int lineno, const char *newpath); ssize_t safe_write(const char *file, const int lineno, - void (cleanup_fn)(void), char len_strict, int fildes, - const void *buf, size_t nbyte); + void (cleanup_fn)(void), enum safe_write_opts len_strict, + int fildes, const void *buf, size_t nbyte); long safe_strtol(const char *file, const int lineno, void (cleanup_fn)(void), char *str, long min, long max); diff --git a/lib/safe_macros.c b/lib/safe_macros.c index 16e582bc976b..eac31f4ce3ff 100644 --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -524,20 +524,42 @@ int safe_symlink(const char *file, const int lineno, } ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void), - char len_strict, int fildes, const void *buf, size_t nbyte) + enum safe_write_opts len_strict, int fildes, const void *buf, + size_t nbyte) { ssize_t rval; + const void *wbuf = buf; + size_t len = nbyte; + int iter = 0; + + do { + iter++; + rval = write(fildes, wbuf, len); + if (rval == -1) { + if (len_strict == SAFE_WRITE_RETRY) + tst_resm_(file, lineno, TINFO, + "write() wrote %zu bytes in %d calls", + nbyte-len, iter); + tst_brkm_(file, lineno, TBROK | TERRNO, + cleanup_fn, "write(%d,%p,%zu) failed", + fildes, buf, nbyte); + } - rval = write(fildes, buf, nbyte); + if (len_strict == SAFE_WRITE_ANY) + return rval; - if (rval == -1 || (len_strict && (size_t)rval != nbyte)) { - tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, - "write(%d,%p,%zu) failed", fildes, buf, nbyte); - } else if (rval < 0) { - tst_brkm_(file, lineno, TBROK | TERRNO, cleanup_fn, - "Invalid write(%d,%p,%zu) return value %zd", fildes, - buf, nbyte, rval); - } + if (len_strict == SAFE_WRITE_ALL) { + if ((size_t)rval != nbyte) + tst_brkm_(file, lineno, TBROK | TERRNO, + cleanup_fn, "short write(%d,%p,%zu) " + "return value %zd", + fildes, buf, nbyte, rval); + return rval; + } + + wbuf += rval; + len -= rval; + } while (len > 0); return rval; } diff --git a/lib/tests/tst_safe_macros.c b/lib/tests/tst_safe_macros.c index b5809f40d10e..5c427ee16832 100644 --- a/lib/tests/tst_safe_macros.c +++ b/lib/tests/tst_safe_macros.c @@ -31,9 +31,9 @@ int main(int argc LTP_ATTRIBUTE_UNUSED, char **argv) printf("buf: %s\n", buf); SAFE_READ(cleanup, 1, fd, buf, 9); printf("buf: %s\n", buf); - SAFE_WRITE(cleanup, 0, -1, buf, 9); - SAFE_WRITE(NULL, 0, fd, buf, 9); - SAFE_WRITE(NULL, 1, fd, buf, 9); + SAFE_WRITE(cleanup, SAFE_WRITE_ANY, -1, buf, 9); + SAFE_WRITE(NULL, SAFE_WRITE_ANY, fd, buf, 9); + SAFE_WRITE(NULL, SAFE_WRITE_ALL, fd, buf, 9); SAFE_PIPE(NULL, fds); return 0; -- 2.27.0 -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-04 8:31 ` [LTP] [PATCH v2] lib: introduce safe_write() retry Jan Stancek @ 2022-10-04 12:20 ` Petr Vorel 2022-10-04 12:34 ` Jan Stancek 2022-10-05 9:32 ` Cyril Hrubis 1 sibling, 1 reply; 13+ messages in thread From: Petr Vorel @ 2022-10-04 12:20 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp Hi Jan, > Turn safe_write() len_strict parameter into 3-way switch, introducing > one additional mode of operation "retry". On short writes, this > resumes write() with remainder of the buffer. Reviewed-by: Petr Vorel <pvorel@suse.cz> > --- > include/safe_macros_fn.h | 11 ++++++++-- > lib/safe_macros.c | 42 ++++++++++++++++++++++++++++--------- > lib/tests/tst_safe_macros.c | 6 +++--- > 3 files changed, 44 insertions(+), 15 deletions(-) LGTM. I just wonder if we need to add it to lib/safe_macros.c, which implements it for the old API. Would it work to add it only to tst_safe_macros.c and tst_safe_macros.h (instead of safe_macros_fn.h)? Not a requirement, just if it makes sense to you. ... > +++ b/include/safe_macros_fn.h > @@ -24,6 +24,13 @@ > #include <unistd.h> > #include <dirent.h> > +/* supported values for safe_write() len_strict parameter */ > +enum safe_write_opts { > + SAFE_WRITE_ANY = 0, // no length strictness, short writes are ok > + SAFE_WRITE_ALL = 1, // strict length, short writes raise TBROK > + SAFE_WRITE_RETRY = 2, // retry/resume after short write > +}; Maybe use /* */ and for readability, maybe put into it's own line? enum safe_write_opts { /* no length strictness, short writes are ok */ SAFE_WRITE_ANY = 0, /* strict length, short writes raise TBROK */ SAFE_WRITE_ALL = 1, /* retry/resume after short write */ SAFE_WRITE_RETRY = 2, // }; Also checkpatch.pl complains: safe_macros_fn.h:29: ERROR: code indent should use tabs where possible safe_macros_fn.h:29: WARNING: please, no spaces at the start of a line safe_macros_fn.h:30: ERROR: code indent should use tabs where possible safe_macros_fn.h:30: WARNING: please, no spaces at the start of a line safe_macros_fn.h:31: ERROR: code indent should use tabs where possible safe_macros_fn.h:31: WARNING: please, no spaces at the start of a line Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-04 12:20 ` Petr Vorel @ 2022-10-04 12:34 ` Jan Stancek 2022-10-04 18:45 ` Petr Vorel 0 siblings, 1 reply; 13+ messages in thread From: Jan Stancek @ 2022-10-04 12:34 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp On Tue, Oct 4, 2022 at 2:20 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Jan, > > > Turn safe_write() len_strict parameter into 3-way switch, introducing > > one additional mode of operation "retry". On short writes, this > > resumes write() with remainder of the buffer. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > --- > > include/safe_macros_fn.h | 11 ++++++++-- > > lib/safe_macros.c | 42 ++++++++++++++++++++++++++++--------- > > lib/tests/tst_safe_macros.c | 6 +++--- > > 3 files changed, 44 insertions(+), 15 deletions(-) > > LGTM. I just wonder if we need to add it to lib/safe_macros.c, It's currently shared code. > which implements it for the old API. Would it work to add it only to > tst_safe_macros.c and tst_safe_macros.h (instead of safe_macros_fn.h)? We could have 2 implementations for safe_write, but modifying existing one seemed better option. There's no harm supporting new option in old API too. > Not a requirement, just if it makes sense to you. > > ... > > +++ b/include/safe_macros_fn.h > > @@ -24,6 +24,13 @@ > > #include <unistd.h> > > #include <dirent.h> > > > +/* supported values for safe_write() len_strict parameter */ > > +enum safe_write_opts { > > + SAFE_WRITE_ANY = 0, // no length strictness, short writes are ok > > + SAFE_WRITE_ALL = 1, // strict length, short writes raise TBROK > > + SAFE_WRITE_RETRY = 2, // retry/resume after short write > > +}; > > Maybe use /* */ and for readability, maybe put into it's own line? > > enum safe_write_opts { > /* no length strictness, short writes are ok */ > SAFE_WRITE_ANY = 0, > > /* strict length, short writes raise TBROK */ > SAFE_WRITE_ALL = 1, > > /* retry/resume after short write */ > SAFE_WRITE_RETRY = 2, // > }; > > Also checkpatch.pl complains: > > safe_macros_fn.h:29: ERROR: code indent should use tabs where possible > safe_macros_fn.h:29: WARNING: please, no spaces at the start of a line > safe_macros_fn.h:30: ERROR: code indent should use tabs where possible > safe_macros_fn.h:30: WARNING: please, no spaces at the start of a line > safe_macros_fn.h:31: ERROR: code indent should use tabs where possible > safe_macros_fn.h:31: WARNING: please, no spaces at the start of a line thanks, I missed that. > > Kind regards, > Petr > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-04 12:34 ` Jan Stancek @ 2022-10-04 18:45 ` Petr Vorel 0 siblings, 0 replies; 13+ messages in thread From: Petr Vorel @ 2022-10-04 18:45 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp Hi Jan, ... > > LGTM. I just wonder if we need to add it to lib/safe_macros.c, > It's currently shared code. Ah, I'm sorry. > > which implements it for the old API. Would it work to add it only to > > tst_safe_macros.c and tst_safe_macros.h (instead of safe_macros_fn.h)? > We could have 2 implementations for safe_write, but modifying existing one > seemed better option. There's no harm supporting new option in old API too. Sure, np. Kind regards, Petr -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-04 8:31 ` [LTP] [PATCH v2] lib: introduce safe_write() retry Jan Stancek 2022-10-04 12:20 ` Petr Vorel @ 2022-10-05 9:32 ` Cyril Hrubis 2022-10-05 12:02 ` Jan Stancek 1 sibling, 1 reply; 13+ messages in thread From: Cyril Hrubis @ 2022-10-05 9:32 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp Hi! Reviewed-by: Cyril Hrubis <chrubis@suse.cz> -- Cyril Hrubis chrubis@suse.cz -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-05 9:32 ` Cyril Hrubis @ 2022-10-05 12:02 ` Jan Stancek 2022-10-17 14:15 ` Richard Palethorpe 0 siblings, 1 reply; 13+ messages in thread From: Jan Stancek @ 2022-10-05 12:02 UTC (permalink / raw) To: Cyril Hrubis; +Cc: ltp On Wed, Oct 5, 2022 at 11:31 AM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Pushed with Petr notes about comments and whitespace fixed. > > -- > Cyril Hrubis > chrubis@suse.cz > -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [LTP] [PATCH v2] lib: introduce safe_write() retry 2022-10-05 12:02 ` Jan Stancek @ 2022-10-17 14:15 ` Richard Palethorpe 0 siblings, 0 replies; 13+ messages in thread From: Richard Palethorpe @ 2022-10-17 14:15 UTC (permalink / raw) To: Jan Stancek; +Cc: ltp Hello, Jan Stancek <jstancek@redhat.com> writes: > On Wed, Oct 5, 2022 at 11:31 AM Cyril Hrubis <chrubis@suse.cz> wrote: >> >> Hi! >> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > > Pushed with Petr notes about comments and whitespace fixed. > >> >> -- >> Cyril Hrubis >> chrubis@suse.cz >> I have set this to accepted in patchwork. Please update patchwork when merging something. -- Thank you, Richard. -- Mailing list info: https://lists.linux.it/listinfo/ltp ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-10-17 14:16 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-03 12:48 [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 2/3] syscalls/preadv203: don't treat short write() as failure Jan Stancek 2022-10-03 12:48 ` [LTP] [PATCH 3/3] mmapstress04: use SAFE_WRITE_FULLY from LTP library Jan Stancek 2022-10-03 13:16 ` [LTP] [PATCH 1/3] lib: introduce safe_write_fully() Cyril Hrubis 2022-10-03 13:56 ` Jan Stancek 2022-10-03 14:00 ` Cyril Hrubis 2022-10-04 8:31 ` [LTP] [PATCH v2] lib: introduce safe_write() retry Jan Stancek 2022-10-04 12:20 ` Petr Vorel 2022-10-04 12:34 ` Jan Stancek 2022-10-04 18:45 ` Petr Vorel 2022-10-05 9:32 ` Cyril Hrubis 2022-10-05 12:02 ` Jan Stancek 2022-10-17 14:15 ` Richard Palethorpe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox