From owner-freebsd-firewire@FreeBSD.ORG  Mon Aug 18 19:14:34 2008
Return-Path: <owner-freebsd-firewire@FreeBSD.ORG>
Delivered-To: freebsd-firewire@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id 57B3A106566B;
	Mon, 18 Aug 2008 19:14:34 +0000 (UTC)
	(envelope-from freebsd@sopwith.solgatos.com)
Received: from parsely.rain.com (parsely.rain.com [199.26.172.196])
	by mx1.freebsd.org (Postfix) with ESMTP id 992A78FC1A;
	Mon, 18 Aug 2008 19:14:33 +0000 (UTC)
	(envelope-from freebsd@sopwith.solgatos.com)
Received: from sopwith.solgatos.com (uucp@localhost)
	by parsely.rain.com (8.11.4/8.11.4) with UUCP id m7IJEPO51816;
	Mon, 18 Aug 2008 12:14:25 -0700 (PDT)
	(envelope-from freebsd@sopwith.solgatos.com)
Received: from localhost by sopwith.solgatos.com (8.8.8/6.24)
	id TAA21449; Mon, 18 Aug 2008 19:13:29 GMT
Message-Id: <200808181913.TAA21449@sopwith.solgatos.com>
To: Sean Bruno <sbruno@miralink.com>
In-reply-to: Your message of "Sun, 17 Aug 2008 10:43:17 PDT."
	<48A86335.8060508@miralink.com> 
Date: Mon, 18 Aug 2008 12:13:29 +0100
From: Dieter <freebsd@sopwith.solgatos.com>
Cc: Scott Long <scottl@samsco.org>, freebsd-firewire@freebsd.org
Subject: Re: fwcontrol update 
X-BeenThere: freebsd-firewire@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Firewire support in FreeBSD <freebsd-firewire.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-firewire>, 
	<mailto:freebsd-firewire-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-firewire>
List-Post: <mailto:freebsd-firewire@freebsd.org>
List-Help: <mailto:freebsd-firewire-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-firewire>, 
	<mailto:freebsd-firewire-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Mon, 18 Aug 2008 19:14:34 -0000

case 'b':

	if (priority_budget < 0 || priority_budget > INT32_MAX)
        	errx(EX_USAGE, "%s: invalid number: %s", __func__, optarg);

case 'f':

	if ( (adjust_gap_count < 0) || (adjust_gap_count > INT32_MAX) )
        	err(EX_USAGE, "%s:adjust_gap_count out of range", __func__);


I think "out of range" is better than "invalid number".
-5 is a valid number.

Just a minor nit, feel free to ignore this one.  :-)

================================================================================

> case 'c':
>           crom_string = malloc(strlen(optarg)+1);
>           if (crom_string == NULL)
>              err(EX_SOFTWARE, "%s:crom_string malloc", __func__);
>           if ( (strtol(crom_string, NULL, 0) < 0) || strtol(crom_string, NULL, 0) > MAX_BOARDS)
>              err(EX_USAGE, "%s:Invalid value for node", __func__);
>           strcpy(crom_string, optarg);

Strtol() reads freshly malloc-ed memory before anything has been put there.

Perhaps:

case 'c':
	{
	long node_num;
	node_num = strtol(optarg, NULL, 0);
	if ( (node_num < 0) || (node_num > MAX_BOARDS) )
		err(EX_USAGE, "%s:node out of range", __func__);
	crom_string = malloc(strlen(optarg)+1);
	if (crom_string == NULL)
		err(EX_SOFTWARE, "%s:crom_string malloc", __func__);
	strcpy(crom_string, optarg);

	...

	}
case 'd':


================================================================================

case 'u':

	current_board = strtol(optarg, NULL, 0);

Does this need a range check?

================================================================================