Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 26 Feb 2001 16:09:07 -0700
From:      "Kenneth D. Merry" <ken@kdm.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        arch@FreeBSD.ORG
Subject:   Re: sbufs in userland
Message-ID:  <20010226160907.A26335@panzer.kdm.org>
In-Reply-To: <Pine.BSF.4.21.0102270336450.19382-100000@besplex.bde.org>; from bde@zeta.org.au on Tue, Feb 27, 2001 at 04:05:34AM %2B1100
References:  <20010226003319.A19994@panzer.kdm.org> <Pine.BSF.4.21.0102270336450.19382-100000@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010226160907.A26335>