From owner-freebsd-hackers@FreeBSD.ORG  Fri Apr  1 11:49:40 2011
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Delivered-To: freebsd-hackers@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 1233)
	id 070CC1065677; Fri,  1 Apr 2011 11:49:40 +0000 (UTC)
Date: Fri, 1 Apr 2011 11:49:40 +0000
From: Alexander Best <arundel@freebsd.org>
To: Sergey Kandaurov <pluknet@freebsd.org>
Message-ID: <20110401114940.GA15680@freebsd.org>
References: <20110331210349.GA4112@freebsd.org>
	<AANLkTinK2u4G9m00RXUKK6C+BAtO7SdNNX8Er+Y4YRNL@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <AANLkTinK2u4G9m00RXUKK6C+BAtO7SdNNX8Er+Y4YRNL@mail.gmail.com>
Cc: freebsd-hackers@freebsd.org, FreeBSD Current <freebsd-current@freebsd.org>
Subject: Re: issue with devstat_buildmatch(3) and certain strings
X-BeenThere: freebsd-hackers@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Technical Discussions relating to FreeBSD
	<freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>, 
	<mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
	<mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 01 Apr 2011 11:49:40 -0000

On Fri Apr  1 11, Sergey Kandaurov wrote:
> On 1 April 2011 01:03, Alexander Best <arundel@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)
> @@ -1014,11 +1014,12 @@
>          * 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); )
                                                                              ^^
                                                                    extra space?
> +               if (**tempstr != '\0') {
> +                       num_args++;
>                         if (++tempstr >= &tstr[5])
>                                 break;
> +               }
> 
>         /* The user gave us too many type arguments */
>         if (num_args > 3) {
> 
> Please review, and I will commit the patch.

looking good. thanks. :)

> 
> -- 
> wbr,
> pluknet

-- 
a13x