[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] dd: add 'fullblock' iflag.
From: Nicholas Clark <nicholas.clark () gmail ! com>
Date: 2018-01-25 18:11:30
Message-ID: CAKNeuBpyHK4cU0VO9hmOnSSGCnktFuz2okRMot2=avuNqS=sAw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Looks great! Do you think we should include a different way to tell the
user that some of their requested data didn't actually get written?
On Thu, Jan 25, 2018 at 11:01 AM, Denys Vlasenko <vda.linux@googlemail.com>
wrote:
> Applied without the code which prints warning.
> Please try current git.
>
> On Wed, Jan 24, 2018 at 7:05 PM, Nicholas Clark
> <nicholas.clark@gmail.com> wrote:
> > Adds a fullblock iflag for improved compatibility with GNU dd.
> > The new iflag can be used to ensure that dd calls retrieve the
> > expected amount of data when reading from pipes or unusual
> > filesystems.
> >
> > Signed-off-by: Nicholas Clark <nicholas.clark@gmail.com>
> > ---
> > coreutils/dd.c | 64 ++++++++++++++++++++++++++++++
> +++++++++-------
> > docs/posix_conformance.txt | 5 ++--
> > 2 files changed, 58 insertions(+), 11 deletions(-)
> >
> > diff --git a/coreutils/dd.c b/coreutils/dd.c
> > index d302f35d3..760420687 100644
> > --- a/coreutils/dd.c
> > +++ b/coreutils/dd.c
> > @@ -37,7 +37,7 @@
> > //config: elapsed time and speed.
> > //config:
> > //config:config FEATURE_DD_IBS_OBS
> > -//config: bool "Enable ibs, obs and conv options"
> > +//config: bool "Enable ibs, obs, iflag and conv options"
> > //config: default y
> > //config: depends on DD
> > //config: help
> > @@ -57,7 +57,7 @@
> >
> > //usage:#define dd_trivial_usage
> > //usage: "[if=FILE] [of=FILE] " IF_FEATURE_DD_IBS_OBS("[ibs=N]
> [obs=N] ") "[bs=N] [count=N] [skip=N]\n"
> > -//usage: " [seek=N]" IF_FEATURE_DD_IBS_OBS("
> [conv=notrunc|noerror|sync|fsync] [iflag=skip_bytes]")
> > +//usage: " [seek=N]" IF_FEATURE_DD_IBS_OBS("
> [conv=notrunc|noerror|sync|fsync] [iflag=skip_bytes|fullblock]")
> > //usage:#define dd_full_usage "\n\n"
> > //usage: "Copy a file with converting and formatting\n"
> > //usage: "\n if=FILE Read from FILE instead of stdin"
> > @@ -79,6 +79,7 @@
> > //usage: "\n conv=fsync Physically write data out before
> finishing"
> > //usage: "\n conv=swab Swap every pair of bytes"
> > //usage: "\n iflag=skip_bytes skip=N is in bytes"
> > +//usage: "\n iflag=fullblock Try to read full blocks from
> input"
> > //usage: )
> > //usage: IF_FEATURE_DD_STATUS(
> > //usage: "\n status=noxfer Suppress rate output"
> > @@ -110,6 +111,10 @@ struct globals {
> > unsigned long long begin_time_us;
> > #endif
> > int flags;
> > +#if ENABLE_FEATURE_DD_IBS_OBS
> > + char warn_en;
> > + char previous_partial;
> > +#endif
> > } FIX_ALIASING;
> > #define G (*(struct globals*)bb_common_bufsiz1)
> > #define INIT_G() do { \
> > @@ -130,11 +135,13 @@ enum {
> > /* start of input flags */
> > FLAG_IFLAG_SHIFT = 5,
> > FLAG_SKIP_BYTES = (1 << 5) * ENABLE_FEATURE_DD_IBS_OBS,
> > + FLAG_FULLBLOCK = (1 << 6) * ENABLE_FEATURE_DD_IBS_OBS,
> > /* end of input flags */
> > - FLAG_TWOBUFS = (1 << 6) * ENABLE_FEATURE_DD_IBS_OBS,
> > - FLAG_COUNT = 1 << 7,
> > - FLAG_STATUS_NONE = 1 << 8,
> > - FLAG_STATUS_NOXFER = 1 << 9,
> > + FLAG_TWOBUFS = (1 << 7) * ENABLE_FEATURE_DD_IBS_OBS,
> > + FLAG_COUNT = 1 << 8,
> > + FLAG_BS = 1 << 9,
> > + FLAG_STATUS_NONE = 1 << 10,
> > + FLAG_STATUS_NOXFER = 1 << 11,
> > };
> >
> > static void dd_output_status(int UNUSED_PARAM cur_signal)
> > @@ -189,6 +196,25 @@ static ssize_t full_write_or_warn(const void *buf,
> size_t len,
> > return n;
> > }
> >
> > +#if ENABLE_FEATURE_DD_IBS_OBS
> > +static ssize_t read_and_warn(int fd, void *buf, size_t count)
> > +{
> > + ssize_t result = safe_read(fd, buf, count);
> > +
> > + if (result <= 0) {
> > + return result;
> > + }
> > +
> > + if (G.previous_partial && G.warn_en) {
> > + G.warn_en = 0;
> > + bb_error_msg("warn: partial read; suggest
> iflag=fullblock");
> > + }
> > +
> > + G.previous_partial = (result < count);
> > + return result;
> > +}
> > +#endif
> > +
> > static bool write_and_stats(const void *buf, size_t len, size_t obs,
> > const char *filename)
> > {
> > @@ -251,7 +277,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > static const char conv_words[] ALIGN1 =
> > "notrunc\0""sync\0""noerror\0""fsync\0""swab\0";
> > static const char iflag_words[] ALIGN1 =
> > - "skip_bytes\0";
> > + "skip_bytes\0""fullblock\0";
> > #endif
> > #if ENABLE_FEATURE_DD_STATUS
> > static const char status_words[] ALIGN1 =
> > @@ -290,6 +316,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > /* Partially implemented: */
> > //swab swap every pair of input bytes: will abort on
> non-even reads
> > OP_iflag_skip_bytes,
> > + OP_iflag_fullblock,
> > #endif
> > };
> > smallint exitcode = EXIT_FAILURE;
> > @@ -347,6 +374,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > if (what == OP_ibs) {
> > /* Must fit into positive ssize_t */
> > ibs = xatoul_range_sfx(val, 1, ((size_t)-1L)/2,
> cwbkMG_suffixes);
> > + G.flags |= FLAG_BS;
> > /*continue;*/
> > }
> > if (what == OP_obs) {
> > @@ -365,6 +393,9 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > if (what == OP_bs) {
> > ibs = xatoul_range_sfx(val, 1, ((size_t)-1L)/2,
> cwbkMG_suffixes);
> > obs = ibs;
> > +#if ENABLE_FEATURE_DD_IBS_OBS
> > + G.flags |= FLAG_BS;
> > +#endif
> > /*continue;*/
> > }
> > /* These can be large: */
> > @@ -405,6 +436,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > ibuf = xmalloc(ibs);
> > obuf = ibuf;
> > #if ENABLE_FEATURE_DD_IBS_OBS
> > + G.warn_en = (G.flags & (FLAG_BS + FLAG_COUNT)) == (FLAG_BS +
> FLAG_COUNT);
> > if (ibs != obs) {
> > G.flags |= FLAG_TWOBUFS;
> > obuf = xmalloc(obs);
> > @@ -450,7 +482,15 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> > size_t blocksz = (G.flags & FLAG_SKIP_BYTES) ? 1 : ibs;
> > if (lseek(ifd, skip * blocksz, SEEK_CUR) < 0) {
> > do {
> > - ssize_t n = safe_read(ifd, ibuf,
> blocksz);
> > + ssize_t n;
> > +#if ENABLE_FEATURE_DD_IBS_OBS
> > + if (G.flags & FLAG_FULLBLOCK)
> > + n = full_read(ifd, ibuf,
> blocksz);
> > + else
> > + n = read_and_warn(ifd, ibuf,
> blocksz);
> > +#else
> > + n = safe_read(ifd, ibuf, blocksz);
> > +#endif
> > if (n < 0)
> > goto die_infile;
> > if (n == 0)
> > @@ -465,8 +505,14 @@ int dd_main(int argc UNUSED_PARAM, char **argv)
> >
> > while (!(G.flags & FLAG_COUNT) || (G.in_full + G.in_part !=
> count)) {
> > ssize_t n;
> > -
> > +#if ENABLE_FEATURE_DD_IBS_OBS
> > + if (G.flags & FLAG_FULLBLOCK)
> > + n = full_read(ifd, ibuf, ibs);
> > + else
> > + n = read_and_warn(ifd, ibuf, ibs);
> > +#else
> > n = safe_read(ifd, ibuf, ibs);
> > +#endif
> > if (n == 0)
> > break;
> > if (n < 0) {
> > diff --git a/docs/posix_conformance.txt b/docs/posix_conformance.txt
> > index 8b9112020..cdf89b744 100644
> > --- a/docs/posix_conformance.txt
> > +++ b/docs/posix_conformance.txt
> > @@ -178,9 +178,10 @@ dd POSIX options:
> > conv=noerror | yes | |
> > conv=notrunc | yes | |
> > conv=sync | yes | |
> > +dd compatibility options:
> > + conv=fsync | yes | |
> > iflag=skip_bytes| yes | |
> > -dd Busybox specific options:
> > - conv=fsync
> > + iflag=fullblock | yes | |
> >
> > df POSIX options
> > option | exists | compliant | remarks
> > --
> > 2.14.1
> >
> > _______________________________________________
> > busybox mailing list
> > busybox@busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox
>
[Attachment #5 (text/html)]
<div dir="ltr">Looks great! Do you think we should include a different way to tell \
the user that some of their requested data didn't actually get \
written?<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan \
25, 2018 at 11:01 AM, Denys Vlasenko <span dir="ltr"><<a \
href="mailto:vda.linux@googlemail.com" \
target="_blank">vda.linux@googlemail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Applied without the code which prints warning.<br> Please try \
current git.<br> <div><div class="h5"><br>
On Wed, Jan 24, 2018 at 7:05 PM, Nicholas Clark<br>
<<a href="mailto:nicholas.clark@gmail.com">nicholas.clark@gmail.com</a>> \
wrote:<br> > Adds a fullblock iflag for improved compatibility with GNU dd.<br>
> The new iflag can be used to ensure that dd calls retrieve the<br>
> expected amount of data when reading from pipes or unusual<br>
> filesystems.<br>
><br>
> Signed-off-by: Nicholas Clark <<a \
href="mailto:nicholas.clark@gmail.com">nicholas.clark@gmail.com</a>><br> > \
---<br> > coreutils/dd.c | 64 \
++++++++++++++++++++++++++++++<wbr>+++++++++-------<br> > \
docs/posix_conformance.txt | 5 ++--<br> > 2 files changed, 58 insertions(+), \
11 deletions(-)<br> ><br>
> diff --git a/coreutils/dd.c b/coreutils/dd.c<br>
> index d302f35d3..760420687 100644<br>
> --- a/coreutils/dd.c<br>
> +++ b/coreutils/dd.c<br>
> @@ -37,7 +37,7 @@<br>
> //config: elapsed time and speed.<br>
> //config:<br>
> //config:config FEATURE_DD_IBS_OBS<br>
> -//config: bool "Enable ibs, obs and conv options"<br>
> +//config: bool "Enable ibs, obs, iflag and conv options"<br>
> //config: default y<br>
> //config: depends on DD<br>
> //config: help<br>
> @@ -57,7 +57,7 @@<br>
><br>
> //usage:#define dd_trivial_usage<br>
> //usage: "[if=FILE] [of=FILE] " \
IF_FEATURE_DD_IBS_OBS("[ibs=N] [obs=N] ") "[bs=N] [count=N] \
[skip=N]\n"<br> > -//usage: " [seek=N]" \
IF_FEATURE_DD_IBS_OBS(" [conv=notrunc|noerror|sync|<wbr>fsync] \
[iflag=skip_bytes]")<br> > +//usage: " \
[seek=N]" IF_FEATURE_DD_IBS_OBS(" [conv=notrunc|noerror|sync|<wbr>fsync] \
[iflag=skip_bytes|fullblock]")<br> > //usage:#define dd_full_usage \
"\n\n"<br> > //usage: "Copy a file with converting and \
formatting\n"<br> > //usage: "\n if=FILE \
Read from FILE instead of stdin"<br> > @@ -79,6 +79,7 @@<br>
> //usage: "\n conv=fsync Physically write data \
out before finishing"<br> > //usage: "\n conv=swab \
Swap every pair of bytes"<br> > //usage: "\n \
iflag=skip_bytes skip=N is in bytes"<br> > +//usage: \
"\n iflag=fullblock Try to read full blocks from input"<br> > \
//usage: )<br> > //usage: IF_FEATURE_DD_STATUS(<br>
> //usage: "\n status=noxfer Suppress rate \
output"<br> > @@ -110,6 +111,10 @@ struct globals {<br>
> unsigned long long begin_time_us;<br>
> #endif<br>
> int flags;<br>
> +#if ENABLE_FEATURE_DD_IBS_OBS<br>
> + char warn_en;<br>
> + char previous_partial;<br>
> +#endif<br>
> } FIX_ALIASING;<br>
> #define G (*(struct globals*)bb_common_bufsiz1)<br>
> #define INIT_G() do { \<br>
> @@ -130,11 +135,13 @@ enum {<br>
> /* start of input flags */<br>
> FLAG_IFLAG_SHIFT = 5,<br>
> FLAG_SKIP_BYTES = (1 << 5) * ENABLE_FEATURE_DD_IBS_OBS,<br>
> + FLAG_FULLBLOCK = (1 << 6) * ENABLE_FEATURE_DD_IBS_OBS,<br>
> /* end of input flags */<br>
> - FLAG_TWOBUFS = (1 << 6) * ENABLE_FEATURE_DD_IBS_OBS,<br>
> - FLAG_COUNT = 1 << 7,<br>
> - FLAG_STATUS_NONE = 1 << 8,<br>
> - FLAG_STATUS_NOXFER = 1 << 9,<br>
> + FLAG_TWOBUFS = (1 << 7) * ENABLE_FEATURE_DD_IBS_OBS,<br>
> + FLAG_COUNT = 1 << 8,<br>
> + FLAG_BS = 1 << 9,<br>
> + FLAG_STATUS_NONE = 1 << 10,<br>
> + FLAG_STATUS_NOXFER = 1 << 11,<br>
> };<br>
><br>
> static void dd_output_status(int UNUSED_PARAM cur_signal)<br>
> @@ -189,6 +196,25 @@ static ssize_t full_write_or_warn(const void *buf, size_t \
len,<br> > return n;<br>
> }<br>
><br>
> +#if ENABLE_FEATURE_DD_IBS_OBS<br>
> +static ssize_t read_and_warn(int fd, void *buf, size_t count)<br>
> +{<br>
> + ssize_t result = safe_read(fd, buf, count);<br>
> +<br>
> + if (result <= 0) {<br>
> + return result;<br>
> + }<br>
> +<br>
> + if (G.previous_partial && G.warn_en) {<br>
> + G.warn_en = 0;<br>
> + bb_error_msg("warn: partial read; suggest \
iflag=fullblock");<br> > + }<br>
> +<br>
> + G.previous_partial = (result < count);<br>
> + return result;<br>
> +}<br>
> +#endif<br>
> +<br>
> static bool write_and_stats(const void *buf, size_t len, size_t obs,<br>
> const char *filename)<br>
> {<br>
> @@ -251,7 +277,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> static const char conv_words[] ALIGN1 =<br>
> \
"notrunc\0""sync\0""noerror\0"<wbr>"fsync\0""swab\0";<br>
> static const char iflag_words[] ALIGN1 =<br>
> - "skip_bytes\0";<br>
> + "skip_bytes\0""fullblock\0";<br>
> #endif<br>
> #if ENABLE_FEATURE_DD_STATUS<br>
> static const char status_words[] ALIGN1 =<br>
> @@ -290,6 +316,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> /* Partially implemented: */<br>
> //swab swap every pair of input bytes: will abort on \
non-even reads<br> > OP_iflag_skip_bytes,<br>
> + OP_iflag_fullblock,<br>
> #endif<br>
> };<br>
> smallint exitcode = EXIT_FAILURE;<br>
> @@ -347,6 +374,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> if (what == OP_ibs) {<br>
> /* Must fit into positive ssize_t */<br>
> ibs = xatoul_range_sfx(val, 1, \
((size_t)-1L)/2, cwbkMG_suffixes);<br> > + \
G.flags |= FLAG_BS;<br> > /*continue;*/<br>
> }<br>
> if (what == OP_obs) {<br>
> @@ -365,6 +393,9 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> if (what == OP_bs) {<br>
> ibs = xatoul_range_sfx(val, 1, \
((size_t)-1L)/2, cwbkMG_suffixes);<br> > obs \
= ibs;<br> > +#if ENABLE_FEATURE_DD_IBS_OBS<br>
> + G.flags |= FLAG_BS;<br>
> +#endif<br>
> /*continue;*/<br>
> }<br>
> /* These can be large: */<br>
> @@ -405,6 +436,7 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> ibuf = xmalloc(ibs);<br>
> obuf = ibuf;<br>
> #if ENABLE_FEATURE_DD_IBS_OBS<br>
> + G.warn_en = (G.flags & (FLAG_BS + FLAG_COUNT)) == (FLAG_BS + \
FLAG_COUNT);<br> > if (ibs != obs) {<br>
> G.flags |= FLAG_TWOBUFS;<br>
> obuf = xmalloc(obs);<br>
> @@ -450,7 +482,15 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
> size_t blocksz = (G.flags & FLAG_SKIP_BYTES) ? 1 : \
ibs;<br> > if (lseek(ifd, skip * blocksz, SEEK_CUR) < \
0) {<br> > do {<br>
> - ssize_t n = safe_read(ifd, ibuf, \
blocksz);<br> > + ssize_t n;<br>
> +#if ENABLE_FEATURE_DD_IBS_OBS<br>
> + if (G.flags & \
FLAG_FULLBLOCK)<br> > + \
n = full_read(ifd, ibuf, blocksz);<br> > + \
else<br> > + n = \
read_and_warn(ifd, ibuf, blocksz);<br> > +#else<br>
> + n = safe_read(ifd, ibuf, \
blocksz);<br> > +#endif<br>
> if (n < 0)<br>
> goto \
die_infile;<br> > if (n == 0)<br>
> @@ -465,8 +505,14 @@ int dd_main(int argc UNUSED_PARAM, char **argv)<br>
><br>
> while (!(G.flags & FLAG_COUNT) || (G.in_full + G.in_part != \
count)) {<br> > ssize_t n;<br>
> -<br>
> +#if ENABLE_FEATURE_DD_IBS_OBS<br>
> + if (G.flags & FLAG_FULLBLOCK)<br>
> + n = full_read(ifd, ibuf, ibs);<br>
> + else<br>
> + n = read_and_warn(ifd, ibuf, ibs);<br>
> +#else<br>
> n = safe_read(ifd, ibuf, ibs);<br>
> +#endif<br>
> if (n == 0)<br>
> break;<br>
> if (n < 0) {<br>
> diff --git a/docs/posix_conformance.txt b/docs/posix_conformance.txt<br>
> index 8b9112020..cdf89b744 100644<br>
> --- a/docs/posix_conformance.txt<br>
> +++ b/docs/posix_conformance.txt<br>
> @@ -178,9 +178,10 @@ dd POSIX options:<br>
> conv=noerror | yes | |<br>
> conv=notrunc | yes | |<br>
> conv=sync | yes | |<br>
> +dd compatibility options:<br>
> + conv=fsync | yes | |<br>
> iflag=skip_bytes| yes | |<br>
> -dd Busybox specific options:<br>
> - conv=fsync<br>
> + iflag=fullblock | yes | |<br>
><br>
> df POSIX options<br>
> option | exists | compliant | remarks<br>
> --<br>
> 2.14.1<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> busybox mailing list<br>
> <a href="mailto:busybox@busybox.net">busybox@busybox.net</a><br>
> <a href="http://lists.busybox.net/mailman/listinfo/busybox" rel="noreferrer" \
target="_blank">http://lists.busybox.net/<wbr>mailman/listinfo/busybox</a><br> \
</blockquote></div><br></div>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic