From owner-p4-projects@FreeBSD.ORG Fri May 30 14:45:28 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id E7ECC37B401; Fri, 30 May 2003 14:45:27 -0700 (PDT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6DCCF37B404 for ; Fri, 30 May 2003 14:45:27 -0700 (PDT) Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by mx1.FreeBSD.org (Postfix) with ESMTP id A255643F85 for ; Fri, 30 May 2003 14:45:26 -0700 (PDT) (envelope-from robert@fledge.watson.org) Received: from fledge.watson.org (localhost [127.0.0.1]) by fledge.watson.org (8.12.9/8.12.9) with ESMTP id h4ULiUOn099300; Fri, 30 May 2003 17:44:30 -0400 (EDT) (envelope-from robert@fledge.watson.org) Received: from localhost (robert@localhost)h4ULiUXj099297; Fri, 30 May 2003 17:44:30 -0400 (EDT) (envelope-from robert@fledge.watson.org) Date: Fri, 30 May 2003 17:44:30 -0400 (EDT) From: Robert Watson X-Sender: robert@fledge.watson.org To: Adam Migus In-Reply-To: <41589.205.227.136.1.1054314700.squirrel@mail.migus.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: amigus@migus.org cc: perforce@freebsd.org Subject: Re: PERFORCE change 32072 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 May 2003 21:45:29 -0000 On Fri, 30 May 2003, Adam Migus wrote: > Ok, so add another int and you can track both lengths. Why do you want > to use sbufs sighting failures when you said you stress tested the code > for three hours and could not make it fail with your redundant fix > present? The test checked for only the first of the two bugs I identified, and with the first in place, indeed the test didn't cause a failure. If the test had checked for the second bug that I later realized also existed, it would have failed. It sounds like we both agree that tracking both lengths is required for the current model, which is probably the final version of the change I'll code up tonight or tomorrow morning. This will involve separating the pointer update from the snprintf() return length check, maintaining two variables, etc. Let me clarify what I mean by an sbuf API change: right now, the fixed size buffer is offered by the MAC Framework to each policy in turn, which appends to the current end of the buffer up to the buffer size. It is the responsibility of each policy performing any write to the buffer to update offsets, check boundary cases, etc. With a single snprintf() for each policy, this was relatively reliable (especially with sample code), and pretty efficient. With many policies performing many updates consisting of many elements, the burden of correct offset calculation, edge case checking, etc, falls on lots of code from diverse providers at many levels. Since C string code is pretty failure-prone (you are far from the first person to bump into nasty failure modes for the snprintf() function, etc), a more robust string handling semantic between the MAC Framework and its policy modules might increase the overall robustness of the framework. Not sure if you've ever used sbufs, but they're being used increasingly for FreeBSD kernel code when generating strings (unfortunately they don't seem to support parsing yet). Basically, they provide a string buffer that auto-grows as elements are added, and has a printf()-like function as one of the growth options. The buffer can be passed from function to function--in fact, this is precisely how GEOM handles asking several modules to update the text string representation of the storage device infrastructure. When the set of updates is completed, sbuf has a method to convert to a flat C string, which would presumably be invoked before any copyout. So the idea would be to modify the externalize API to invoke each policy with: int mpo_externalize(struct label *label, const char *element_name, struct sbuf *sb, int *claimed); The MAC Framework would initialize the sbuf with the size of the eventual user buffer. Rather than dealing with all the pointer, offset, etc, junk, each policy would just call a series of the following calls to append: if (sbuf_printf(sb, "%s...", element_substring, ...) == -1) return (ESOMETHINGNASTYHAPPENED); Sbuf's support both fixed size buffers and auto-extension of the buffer using a blocking malloc. When the MAC Framework is done querying each policy for its updates to the externalized string, it would call sbuf_finish(sb), and extract the C string using sbuf_data(sb). Hopefully, the end result of such a change would be a dramatically reduced chance of buffer related errors, since this approach is more resistant to inconsistent string handling semantics across platforms, etc. Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert@fledge.watson.org Network Associates Laboratories