Re: CVS commit: src/usr.bin/printf
On Sun, Jun 28, 2020 at 21:52:23 +0700, Robert Elz wrote: > Date:Fri, 26 Jun 2020 22:05:05 + > From:"Valeriy E. Ushakov" > Message-ID: <20200626220505.e9030f...@cvs.netbsd.org> > > | Modified Files: > | src/usr.bin/printf: printf.1 > | > | Log Message: > | Drop redundant quoting in the nested printf example. > > I don't think that is correct, the quotes around 0x0A were obviously > redundant (and changing it to 0x0a doesn't alter that), but those > around the $(printf ...) are not - without those the result from the > command substitution would be subject to field splitting, and if IFS > happens to contain a \ or an octal digit, bad things will happen. > > In general it is never a good idea to omit quotes around sh word > expansions except in the cases where you actually want field splitting > (or pathname expansion, etc) to happen (and sometimes except in the > contexts where those don't occur anyway, like x=$whatever) > > For the example in question, try running it with IFS=1 or IFS=2 > to see what I mean. Right, but I'd expect people that actually use IFS with octal digits or a backslash to also understand and know how to add necessary quoting in that case. Though perhaps it makes sense to them back for pedagogical purposes. -uwe
Re: CVS commit: src/usr.bin/printf
Date:Fri, 26 Jun 2020 22:05:05 + From:"Valeriy E. Ushakov" Message-ID: <20200626220505.e9030f...@cvs.netbsd.org> | Modified Files: | src/usr.bin/printf: printf.1 | | Log Message: | Drop redundant quoting in the nested printf example. I don't think that is correct, the quotes around 0x0A were obviously redundant (and changing it to 0x0a doesn't alter that), but those around the $(printf ...) are not - without those the result from the command substitution would be subject to field splitting, and if IFS happens to contain a \ or an octal digit, bad things will happen. In general it is never a good idea to omit quotes around sh word expansions except in the cases where you actually want field splitting (or pathname expansion, etc) to happen (and sometimes except in the contexts where those don't occur anyway, like x=$whatever) For the example in question, try running it with IFS=1 or IFS=2 to see what I mean. kre
Re: CVS commit: src/sys/compat/sys
> On Jun 28, 2020, at 10:24 AM, Robert Elz wrote: > >Date:Sat, 27 Jun 2020 11:49:30 -0400 >From:"Christos Zoulas" >Message-ID: <20200627154930.84e22f...@cvs.netbsd.org> > > | Modified Files: > |src/sys/compat/sys: mount.h > | > | Log Message: > | Ignore the supplied size, and always use the argument size that we know. > > Is this fix correct?Certainly looks more reasonable than > what was there before, as the supplied size (for no seemingly > good reason) is often 0, but in compat_20_sys_getfsstat() (in > sys/compat/common/vfs_syscalls_20.c) statvfs_to_statfs12_copy() > is called, via do_sys_getvfsstat() with a size of > sizeof(struct statvfs90) and a statvfs90 (compat/sys/statvfs.h) > is certainly not the same size as a statfs12 (compat/sys/mount.h) > > Or perhaps (probably more likely) compat_20_sys_getfsstat() should > be passing sizeof(struct statfs12) instead ? (And now may as well > just pass 0). No, it has to be sizeof(struct statfs12) because the function fills an array of struct statfs12 and needs to know how much to increment. Thanks, christos signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/compat/sys
Date:Sat, 27 Jun 2020 11:49:30 -0400 From:"Christos Zoulas" Message-ID: <20200627154930.84e22f...@cvs.netbsd.org> | Modified Files: | src/sys/compat/sys: mount.h | | Log Message: | Ignore the supplied size, and always use the argument size that we know. Is this fix correct?Certainly looks more reasonable than what was there before, as the supplied size (for no seemingly good reason) is often 0, but in compat_20_sys_getfsstat() (in sys/compat/common/vfs_syscalls_20.c) statvfs_to_statfs12_copy() is called, via do_sys_getvfsstat() with a size of sizeof(struct statvfs90) and a statvfs90 (compat/sys/statvfs.h) is certainly not the same size as a statfs12 (compat/sys/mount.h) Or perhaps (probably more likely) compat_20_sys_getfsstat() should be passing sizeof(struct statfs12) instead ? (And now may as well just pass 0). kre