Date: Fri, 30 May 2003 18:30:24 -0400 (EDT) From: "Adam Migus" <adam@migus.org> To: <rwatson@freebsd.org> Cc: perforce@freebsd.org Subject: Re: PERFORCE change 32072 for review Message-ID: <19614.205.227.136.1.1054333824.squirrel@mail.migus.org> In-Reply-To: <Pine.NEB.3.96L.1030530173335.27495a-100000@fledge.watson.org> References: <41589.205.227.136.1.1054314700.squirrel@mail.migus.org> <Pine.NEB.3.96L.1030530173335.27495a-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Great. The only thing I don't understand about this post is why it's addressed to me given my previous post. Please address further discussion of this matter to the perforce mailing list or some other interested party. Thanks. Adam <quote who="Robert Watson"> > 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 -- Adam Migus - Research Scientist Network Associates Laboratories (http://www.nailabs.com)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19614.205.227.136.1.1054333824.squirrel>