From owner-freebsd-current  Tue Nov 23 23:53: 7 1999
Delivered-To: freebsd-current@freebsd.org
Received: from rover.village.org (rover.village.org [204.144.255.49])
	by hub.freebsd.org (Postfix) with ESMTP
	id B6E991501B; Tue, 23 Nov 1999 23:53:01 -0800 (PST)
	(envelope-from imp@harmony.village.org)
Received: from harmony.village.org (harmony.village.org [10.0.0.6])
	by rover.village.org (8.9.3/8.9.3) with ESMTP id AAA12040;
	Wed, 24 Nov 1999 00:50:33 -0700 (MST)
	(envelope-from imp@harmony.village.org)
Received: from harmony.village.org (localhost.village.org [127.0.0.1]) by harmony.village.org (8.9.3/8.8.3) with ESMTP id AAA18917; Wed, 24 Nov 1999 00:50:53 -0700 (MST)
Message-Id: <199911240750.AAA18917@harmony.village.org>
To: Brian Fundakowski Feldman <green@FreeBSD.ORG>
Subject: Re: Overflow in banner(1) 
Cc: Kris Kennaway <kris@hub.freebsd.org>, current@FreeBSD.ORG
In-reply-to: Your message of "Wed, 24 Nov 1999 00:44:11 EST."
		<Pine.BSF.4.10.9911240033221.40905-100000@green.dyndns.org> 
References: <Pine.BSF.4.10.9911240033221.40905-100000@green.dyndns.org>  
Date: Wed, 24 Nov 1999 00:50:53 -0700
From: Warner Losh <imp@village.org>
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

In message <Pine.BSF.4.10.9911240033221.40905-100000@green.dyndns.org> Brian Fundakowski Feldman writes:
: I'd prefer something like this that I've attached.  The move over the
: years has been away from artificial limits...
: @@ -1058,14 +1058,24 @@
:  
:  	/* Have now read in the data. Next get the message to be printed. */
:  	if (*argv) {
: -		strcpy(message, *argv);
: +		message = strdup(*argv);
: +		if (message == NULL)
: +			err(1, "strdup");
:  		while (*++argv) {
: -			strcat(message, " ");
: -			strcat(message, *argv);
: +			char *omessage;
: +
: +			omessage = message;
: +			asprintf(&message, "%s %s", message, *argv);
: +			if (message == NULL)
: +				err(1, "asprintf");
: +			free(omessage);
:  		}
:  		nchars = strlen(message);
:  	} else {
:  		fprintf(stderr,"Message: ");
: +		message = malloc(MAXMSG);
: +		if (message == NULL)
: +			err(1, "malloc");
:  		(void)fgets(message, sizeof(message), stdin);
:  		nchars = strlen(message);
:  		message[nchars--] = '\0';	/* get rid of newline */

I'd have used the realloc primitive in place of the asprintf primitive
here to avoid too many calls to realloc/malloc.  Actually, it is
clearer and easier to understand if you precompute the length first
and malloc enough space.

	len = argc;	 (one each space, plus null at end)
	for (i = 1; i <= argc; i++)
		len += strlen(argv[i]);
	message = malloc(len);
	// original code here, maybe marked with /* XXX SAFE XXX */
	// to show that the code was reviewed at least once.

Warner



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message