Re: [PATCH 3/4] Easy alloca() replacements

[Valerie Aurora Henson <vaurora@redhat.com>]

On Sat, Jan 10, 2009 at 12:18:36PM +0900, Ian Kent wrote:
> On Fri, 2009-01-09 at 13:47 -0500, Valerie Aurora Henson wrote:
> 
> Just a first pass reading but noticed ....
> 
> > diff --git a/lib/cache.c b/lib/cache.c
> > index 36b8294..bc2c722 100644
> > --- a/lib/cache.c
> > +++ b/lib/cache.c
> > @@ -484,27 +484,19 @@ struct mapent *cache_lookup_offset(const char *prefix, const char *offset, int s
> >  {
> >  	struct list_head *p;
> >  	struct mapent *this;
> > -	int plen = strlen(prefix);
> > -	char *o_key;
> > +	char o_key[KEY_MAX_LEN];
> 
> I know this looks right but I think it will end up being a problem
> latter. I know there are places in the map lookup libraries that
> restrict these to KEY_MAX_LEN, which is needed for indirect map keys,
> but direct map keys really shouldn't have this restriction as they
> should be able to be as long as PATH_MAX but that brings the issue of
> needing to audit and adjust the maximum parse buffer as well. So we will
> need to get around to that at some point as well.

It certainly won't hurt to make this PATH_MAX - I'll do that unless I
hear otherwise.

> >  
> >  	/* root offset duplicates "/" */
> > -	if (plen > 1) {
> > -		o_key = alloca(plen + strlen(offset) + 1);
> > -		strcpy(o_key, prefix);
> > -		strcat(o_key, offset);
> > -	} else {
> > -		o_key = alloca(strlen(offset) + 1);
> > -		strcpy(o_key, offset);
> > -	}
> > +	if (snprintf(o_key, sizeof(o_key), "%s%s", prefix, offset) >
> > +	    sizeof(o_key))
> > +		return NULL;
> 
> Looks like this will set o_key to "//..." for strlen(prefix) == 1.

I'll fix it, thanks

-VAL