Re: numfmt (=print 'human' sizes) updates
On 12/14/2012 08:53 AM, Pádraig Brady wrote: On 12/14/2012 05:38 AM, Assaf Gordon wrote: Hello, Attached is a slightly improved patch - minor code changes, and many more tests. Line coverage is 98%, and branch coverage is now 93% , and most of the non-covered branches are simply unreachable (I'm checking the reachable ones). The comments below still apply. It's looking like a really useful cohesive command. I verified all the new tests pass and syntax check is fine too. Your attention to detail is _much_ appreciated. I'll note here some _future_ TODO items, which don't need to be added for the initial version: 1. Parse numbers containing grouping separators. 2. Have --fields support multiple values. I did some basic perf testing and verified no leaks with: $ yes 1 2 | pv /dev/null ^C09MB 0:00:03 [ 102MB/s] [ = ] $ yes 1 2 | (ulimit -v 2; src/numfmt --field 2 --to=iec --from=auto) | pv /dev/null ^C.9MB 0:00:11 [6.32MB/s] [ = ] $ yes 1 2 | head -n10 | valgrind src/numfmt --field 2 --to=iec --from=auto 21 | grep leaks ==23446== All heap blocks were freed -- no leaks are possible I notice the UNIT options: seciont is mis-formatted by help2man. I notice use of fprintf (stderr, ...) where error (0, 0, ...) should be used. indent -nut src/numfmt.c makes lots of changes, most valid. So on to some usage... This shows auto padding works, with before and after displayed through tee. $ df | head -n3 | tee (src/numfmt --header --field=2 --to=iec) Filesystem 1K-blocks Used Available Use% Mounted on rootfs13102208 12544324424816 97% / udev 1454732 0 1454732 0% /dev Filesystem 1K-blocks Used Available Use% Mounted on rootfs 13M 12544324424816 97% / udev 1.4M 0 1454732 0% /dev This shows padding increases the field width as required in edge cases: $ src/df /home | tee (src/numfmt --header --field=2 --from-unit=1024 --to-unit=1 --group) Filesystem 1K-blocks Used Available Use% Mounted on /dev/sdb1 100791728 71565732 24105996 75% /home Filesystem 1K-blocks Used Available Use% Mounted on /dev/sdb1 103,210,729,472 71565732 24105996 75% /home I notice the unit on both --to=iec and --to=si is the same. I was thinking that --to=iec would add a 'i' to distinguish? $ /bin/ls -l | head -n4 | src/numfmt --to=iec --field=5 total 7252 -rw-rw-r--. 1 padraig padraig 92K Aug 23 2011 ABOUT-NLS -rw-rw-r--. 1 padraig padraig 49K Dec 21 16:22 aclocal.m4 -rw-rw-r--. 1 padraig padraig3.6K Dec 21 16:15 AUTHORS I notice we exit immediately with invalid numbers. How about printing the warning, outputting the unparsable number as is and continuing? $ printf %s\n 1 one 2 | src/numfmt 1 src/numfmt: invalid number: 'one' As suggested missing fields are ignored now... $ echo -ne 1\n2 2\n | src/numfmt --field=2 1 2 2 ...and we get warnings with --debug. However they mix awkwardly with the output. Perhaps delay the warning until line causing the warning is displayed. $ echo -ne 1\n2 2\n | src/numfmt --field=2 --debug src/numfmt: no conversion option specified 1src/numfmt: input line is too short, no numbers found to convert in field 2 2 2 --devdebug should probably renamed to ---devdebug I'm starting to think the original idea of having a --format option would be more a more concise, well known and extendible interface rather than having --padding, --grouping, --base, ... It wouldn't have to be passed directly to printf, and could be parsed and preprocessed something like we do in seq(1). thanks! Pádraig.
Re: numfmt (=print 'human' sizes) updates
Hello Pádraig, Thanks for the review and the feedback. Pádraig Brady wrote, On 12/21/2012 12:42 PM: It's looking like a really useful cohesive command. The attached patch addresses the following issues: 1. changed the ---devdebug option 2. incorporated most of 'indent -nut' recommendations. 3. improved the usage string, to generate somewhat better man page. (help2man is a bit finicky about formatting, so it's not optimal). 4. i suffix with iec input/output: I understand the reason for always adding i, as it is The Right Thing to do. But I think some (most?) people still treat single-letter suffix (K/M/G/T/etc.) as a valid suffix for both SI and IEC, and deduce the scale from the context of whatever they're working on. Forcing them to use Ki might be too obtrusive. It could also mess-up automated scripted, when IEC scale is needed, but only with single-letter suffix (and there are many programs like that). As a compromise, I've added yet another scale option: ieci . When used with --from=ieci, a two-letter suffix is required. When used with --to=ieci, i will always be appended. Examples: $ ./src/numfmt --to=iec 4096 4.0K $ ./src/numfmt --to=ieci 4096 4.0Ki $ ./src/numfmt --from=iec 4K 4096 $ ./src/numfmt --from=ieci 4Ki 4096 $ ./src/numfmt --from=auto 4Ki 4096 $ ./src/numfmt --from=auto 4K 4000 # 'ieci' requires the 'i': $ ./src/numfmt --from=ieci 4K ./src/numfmt: missing 'i' suffix in input: '4K' (e.g Ki/Mi/Gi) # 'iec' does not accept 'i': $ ./src/numfmt --from=iec 4Ki ./src/numfmt: invalid suffix in input '4Ki': 'i' I hope this cover all the options, while maintaining consistent and expected behavior. (Optionally, we can change iec to behave like ieci, and rename ieci to something else). I will send --format and error message updates soon. Regards, -gordon numfmt.9.patch.xz Description: application/xz
Re: numfmt (=print 'human' sizes) updates
On 12/22/2012 12:51 AM, Assaf Gordon wrote: Hello Pádraig, Thanks for the review and the feedback. Pádraig Brady wrote, On 12/21/2012 12:42 PM: It's looking like a really useful cohesive command. The attached patch addresses the following issues: 1. changed the ---devdebug option 2. incorporated most of 'indent -nut' recommendations. 3. improved the usage string, to generate somewhat better man page. (help2man is a bit finicky about formatting, so it's not optimal). Yes it's awkward to work around the help2man constraints. 4. i suffix with iec input/output: I understand the reason for always adding i, as it is The Right Thing to do. But I think some (most?) people still treat single-letter suffix (K/M/G/T/etc.) as a valid suffix for both SI and IEC, and deduce the scale from the context of whatever they're working on. Forcing them to use Ki might be too obtrusive. It could also mess-up automated scripted, when IEC scale is needed, but only with single-letter suffix (and there are many programs like that). As a compromise, I've added yet another scale option: ieci . When used with --from=ieci, a two-letter suffix is required. When used with --to=ieci, i will always be appended. Examples: $ ./src/numfmt --to=iec 4096 4.0K $ ./src/numfmt --to=ieci 4096 4.0Ki $ ./src/numfmt --from=iec 4K 4096 $ ./src/numfmt --from=ieci 4Ki 4096 $ ./src/numfmt --from=auto 4Ki 4096 $ ./src/numfmt --from=auto 4K 4000 # 'ieci' requires the 'i': $ ./src/numfmt --from=ieci 4K ./src/numfmt: missing 'i' suffix in input: '4K' (e.g Ki/Mi/Gi) # 'iec' does not accept 'i': $ ./src/numfmt --from=iec 4Ki ./src/numfmt: invalid suffix in input '4Ki': 'i' I hope this cover all the options, while maintaining consistent and expected behavior. (Optionally, we can change iec to behave like ieci, and rename ieci to something else). I like the 'ieci' compromise. Also because it has two 'i's in it, there will be less confusion as to which is which between 'si' and 'ieci'. I suppose an alternative to 'ieci' is 'ibi', but I slightly prefer your 'ieci'. I will send --format and error message updates soon. thanks! Pádraig.
Re: numfmt (=print 'human' sizes) updates
On 12/21/2012 07:07 PM, Pádraig Brady wrote: As a compromise, I've added yet another scale option: ieci . When used with --from=ieci, a two-letter suffix is required. When used with --to=ieci, i will always be appended. That's a little hard to read. Would --from=iec-i be any easier, where we are separating the IEC acronym from the output of 'i'? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
bug#13243: [PATCH] enhancement: modify md5sum to allow piping
Eric Blake wrote: In your case, you can do: dd if=/dev/sda3 | pbzip2 -c2 | tee (md5sum /tmp/sda3.dat.bzip2.md5) | netcat 192.168.1.123 45678 I would also mention the GNU dd extension status=none which avoids what is probably the unnecessary for this purpose records in / records out status output. dd status=none if=/dev/sda3 | ... Bob
