Re: Possible unsafe use of strncat in sbin/sysctl/sysctl.c

[Marc Balmer <mbalmer%NetBSD.org@localhost>]

Am 23.08.11 23:44, schrieb Jimmy Johansson:
> On Tue, Aug 23, 2011 at 11:29:32PM +0200, Marc Balmer wrote:
>> Am 23.08.11 23:23, schrieb Jimmy Johansson:
>>> Hi,
>>>
>>>
> 
> [snip, patch that makes eyes bleed]
> 
>>
>> I don't like how functions, which return values are ignored, are casted
>> to (void).
>>
>> Who invented that idiom?  What is that good for, other than making your
>> eyes bleed?
>>
>> I think it is no more than quieting cc or lint.
> 
> I don't care either way, but it seems to be the suggested way to write
> code for NetBSD according to KFN. I even think that it mentioned casting
> return values from functions where you didn't handle the return value
> specifically at some point, but it doesn't do so now.
> 
> I don't want the eyes of anymore NetBSD developer to bleed though, so
> new patch. :-)

(void)wait!  That was a pure personal remark.  I really don't know where
that (void)bla idiom comes from, and I obviously don't (void)like it.

Some folks seems to (void)love it.

> 
> Index: sbin/sysctl/sysctl.c
> ===================================================================
> RCS file: /cvsroot/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.136
> diff -u -w -p -r1.136 sysctl.c
> --- sbin/sysctl/sysctl.c        3 Aug 2011 01:58:30 -0000       1.136
> +++ sbin/sysctl/sysctl.c        23 Aug 2011 21:41:15 -0000
> @@ -579,8 +579,8 @@ print_tree(int *name, u_int namelen, str
>                 for (ni = 0; ni < namelen; ni++) {
>                         (void)snprintf(n, sizeof(n), "%d", name[ni]);
>                         if (ni > 0)
> -                               strncat(gdname, ".", sizeof(gdname));
> -                       strncat(gdname, n, sizeof(gdname));
> +                               strlcat(gdname, ".", sizeof(gdname));
> +                       strlcat(gdname, n, sizeof(gdname));
>                 }
>         }
>  
> @@ -589,11 +589,11 @@ print_tree(int *name, u_int namelen, str
>         else if (add) {
>                 snprintf(n, sizeof(n), "%d", pnode->sysctl_num);
>                 if (namelen > 1) {
> -                       strncat(gsname, sep, sizeof(gsname));
> -                       strncat(gdname, ".", sizeof(gdname));
> +                       strlcat(gsname, sep, sizeof(gsname));
> +                       strlcat(gdname, ".", sizeof(gdname));
>                 }
> -               strncat(gsname, pnode->sysctl_name, sizeof(gsname));
> -               strncat(gdname, n, sizeof(gdname));
> +               strlcat(gsname, pnode->sysctl_name, sizeof(gsname));
> +               strlcat(gdname, n, sizeof(gdname));
>         }
>  
>         if (Mflag && pnode != &my_root) {
> 
> This is uniform with the other strlcat in the file too, so it might be
> for the better.
> 
> Regards,
> 
> Jimmy


-- 
  \~~~~~.                The NetBSD Foundation
   \~~~~~'               Marc Balmer, Developer / Marketing
  NetBSD
     \                   mbalmer%NetBSD.org@localhost   http://www.NetBSD.org/