Re: CVS commit: src/usr.bin/printf

2020-06-28 Thread Valery Ushakov
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

2020-06-28 Thread Robert Elz
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

2020-06-28 Thread Christos Zoulas


> 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

2020-06-28 Thread Robert Elz
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