Re: issue with devstat_buildmatch(3) and certain strings

On 1 April 2011 15:37, Sergey Kandaurov <pluknet_at_freebsd.org> wrote:
> On 1 April 2011 01:03, Alexander Best <arundel_at_freebsd.org> wrote:
>> hi there,
>>
>> devstat_buildmatch(3) crashes with certain strings. you can test this by
>> doing one of:
>>
>> iostat -t ","
>> iostat -t ",,"
>> iostat -t "da,"
>> iostat -t ",da,"
>> iostat -t ",da"
>> iostat -t "da,scsi,"
>> iostat -t ",da,scsi"
>> iostat -t "da,,scsi"
>
> [Someone told me, -hackers isn't appropriate for patches, Cc: -current.]
>
> The problem is devstat(3) increments num_args regardless if strsep
> returned NULL.
> I think that should work (all your tests pass):
>
> Index: lib/libdevstat/devstat.c
> ===================================================================
> --- lib/libdevstat/devstat.c    (revision 220102)
> +++ lib/libdevstat/devstat.c    (working copy)
> _at__at_ -1014,11 +1014,12 _at__at_
>         * Break the (comma delimited) input string out into separate strings.
>         */
>        for (tempstr = tstr, num_args  = 0;
> -            (*tempstr = strsep(&match_str, ",")) != NULL && (num_args < 5);
> -            num_args++)
> -               if (**tempstr != '\0')
> +            (*tempstr = strsep(&match_str, ",")) != NULL && (num_args < 5); )
> +               if (**tempstr != '\0') {
> +                       num_args++;
>                        if (++tempstr >= &tstr[5])
                               ^^^^
BTW,
this game with pointers might prevent devstat(3) from work on big-endian.

>                                break;
> +               }
>
>        /* The user gave us too many type arguments */
>        if (num_args > 3) {
>
> Please review, and I will commit the patch.


-- 
wbr,
pluknet