Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Dec 2000 13:58:15 -0800
From:      Jake Burkholder <jburkhol@home.com>
To:        Dag-Erling Smorgrav <des@ofug.org>
Cc:        arch@FreeBSD.ORG
Subject:   Re: Safe string formatting in the kernel 
Message-ID:  <20001211215816.1E73EBA7D@io.yi.org>
In-Reply-To: Message from Dag-Erling Smorgrav <des@ofug.org>  of "11 Dec 2000 20:13:24 %2B0100." <xzpvgsq22ln.fsf@flood.ping.uio.no> 

next in thread | previous in thread | raw e-mail | index | archive | help
> Dag-Erling Smorgrav <des@ofug.org> writes:
> >     http://people.freebsd.org/~des/software/sbuf-20001211.diff
> 
> That patch has a bug. I put up a new one:
> 
>     http://people.freebsd.org/~des/software/sbuf-20001211b.diff
> 
> DES
> -- 
> Dag-Erling Smorgrav - des@ofug.org
> 
> 
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-arch" in the body of the message

This is interesting.

Some comments:

+MALLOC_DEFINE(sbuf_malloc_type, "sbuf", "string buffers");

Aren't malloc types usually capital letters that look like a flag?
like M_SBUF?

+struct sbuf {
+       char            *s_buf;         /* storage buffer */
+       size_t           s_size;        /* size of storage buffer */
+       size_t           s_len;         /* current length of string */
+       int              s_dynamic;     /* storage buffer must be freed */
+       int              s_done;        /* sbuf is finished and read-only */
+       int              s_flags;       /* flags */
+       struct sbuf     *s_next;        /* next in chain */
+};

Why not make s_dynamic live in s_flags?  Maybe s_done also?
Maybe also have a flag S_INITED instead of using s_buf as a flag?

If you're going to make a list shouldn't you use the queue(3) macros?

+sbuf_putc(struct sbuf *s, int c)
...
+       s->s_buf[s->s_len] = 0;

'\0' ?

+sbuf_printf(struct sbuf *s, char *fmt, ...)
...
+       len = kvprintf(fmt, _sbuf_pchar, s, 10, ap);

If the idea is to invent our own formats, and sbuf_printf calls kvprintf,
then the new formats need to be added to kvprintf right?  In that case
one would be able to use them with just normal kernel printf which seems
like something we're trying to avoid.  Maybe some kind of printf format
switch table would be useful?  Like an array of function pointers that
would get passed to kvprintf, where the ascii value of the format
character is the index of a function to parse the format.

Jake



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?20001211215816.1E73EBA7D>