From owner-freebsd-arch Mon Feb 26 15: 9:36 2001 Delivered-To: freebsd-arch@freebsd.org Received: from panzer.kdm.org (panzer.kdm.org [216.160.178.169]) by hub.freebsd.org (Postfix) with ESMTP id 4DE1E37B65D for ; Mon, 26 Feb 2001 15:09:16 -0800 (PST) (envelope-from ken@panzer.kdm.org) Received: (from ken@localhost) by panzer.kdm.org (8.9.3/8.9.1) id QAA26427; Mon, 26 Feb 2001 16:09:07 -0700 (MST) (envelope-from ken) Date: Mon, 26 Feb 2001 16:09:07 -0700 From: "Kenneth D. Merry" To: Bruce Evans Cc: arch@FreeBSD.ORG Subject: Re: sbufs in userland Message-ID: <20010226160907.A26335@panzer.kdm.org> References: <20010226003319.A19994@panzer.kdm.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2i In-Reply-To: ; from bde@zeta.org.au on Tue, Feb 27, 2001 at 04:05:34AM +1100 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Tue, Feb 27, 2001 at 04:05:34 +1100, Bruce Evans wrote: > On Mon, 26 Feb 2001, Kenneth D. Merry wrote: > > > ... > > I would like to use the sbuf(9) interface to do the string formatting, > > since it is fairly superior to my current string formatting method (see > > scsi_sense_string() in sys/cam/scsi/scsi_all.c). > > ... > > > Code without sbufs: > > > > switch (error_code) { > > case SSD_DEFERRED_ERROR: > > retlen = snprintf(tmpstr, tmpstrlen, "Deferred Error: "); > > > > if ((tmplen = str_len - cur_len - 1) < 0) > > goto sst_bailout; > > > > strncat(str, tmpstr, tmplen); > > cur_len += retlen; > > str[str_len - 1] = '\0'; > > /* FALLTHROUGH */ > > This seems to be insufficiently large to be correct :-). It doesn't check > for snprintf failing (retlen == -1) or truncating (retlen >= tmpstrlen). True, something like this might do the trick: switch (error_code) { case SSD_DEFERRED_ERROR: retlen = snprintf(tmpstr, tmpstrlen, "Deferred Error: "); if (((tmplen = str_len - cur_len - 1) < 0) || (retlen == -1)) goto sst_bailout; strncat(str, tmpstr, tmplen); cur_len += min(retlen, tmpstrlen); str[str_len - 1] = '\0'; /* FALLTHROUGH */ I think failures are unlikely -- tmpstr is generally long enough to handle anything thrown at it, and I think most of the cases that would cause snprintf() to return -1 are unlikely with our code. The most likely scenario that would cause it would be some sort of integer conversion overflow. In any case, if we end up going with the snprintf version of things instead of sbufs, I'll put the above fixes in to make sure things are correct. > > > > Code with sbufs: > > > > switch (error_code) { > > case SSD_DEFERRED_ERROR: > > sbuf_printf(sb, "Deferred Error: "); > > > > /* FALLTHROUGH */ > > Code with an funopen(3)'d stream: > > switch (error_code) { > case SSD_DEFERRED_ERROR: > fprintf(fp, "Deferred Error: "); > > /* FALLTHROUGH */ > > :-). > > funopen() is more general than sbufs, so it is not quite as easy to use, > but I think it is easy enough. As Poul-Henning pointed out, it would need to be available in the kernel as well as userland in order to accomplish the goal of getting rid of functionally duplicated code. Ken -- Kenneth Merry ken@kdm.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message