From owner-freebsd-arch Mon Dec 11 13:58:22 2000 From owner-freebsd-arch@FreeBSD.ORG Mon Dec 11 13:58:19 2000 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from io.yi.org (unknown [24.70.218.157]) by hub.freebsd.org (Postfix) with ESMTP id 18C9937B402 for ; Mon, 11 Dec 2000 13:58:19 -0800 (PST) Received: from io.yi.org (localhost.gvcl1.bc.wave.home.com [127.0.0.1]) by io.yi.org (Postfix) with ESMTP id 1E73EBA7D; Mon, 11 Dec 2000 13:58:16 -0800 (PST) X-Mailer: exmh version 2.1.1 10/15/1999 To: Dag-Erling Smorgrav Cc: arch@FreeBSD.ORG Subject: Re: Safe string formatting in the kernel In-Reply-To: Message from Dag-Erling Smorgrav of "11 Dec 2000 20:13:24 +0100." Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Mon, 11 Dec 2000 13:58:15 -0800 From: Jake Burkholder Message-Id: <20001211215816.1E73EBA7D@io.yi.org> Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG > Dag-Erling Smorgrav 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