[coreboot] libpayload: Add LAR walking support

[Jordan Crouse]

On 26/04/08 12:22 +0200, Peter Stuge wrote:
> On Fri, Apr 25, 2008 at 05:22:01PM -0600, Jordan Crouse wrote:
> > +struct LAR *openlar(void *addr)
> ..
> > +	lar = calloc(sizeof(struct LAR), 1);
> > +
> > +	if (lar)
> > +		lar->start = addr;
> > +
> > +	/* Preallocate 16 slots in the cache - this saves wear and
> > +	 * tear on the heap */
> > +
> > +	lar->headers = malloc(16 * sizeof(void *));
> > +	lar->alloc = 16;
> > +	lar->count = lar->eof = 0;
> > +	lar->cindex = 0;
> > +
> > +	return lar;
> > +}
> 
> This is not nice if calloc() fails. Maybe add
> 
> if (!lar)
>   return lar;
> 
> instead of the if(lar) before lar->start=addr ?

Fixed.

> > +int closelar(struct LAR *lar)
> > +{
> > +	if (lar) {
> > +		if (lar->headers)
> > +			free(lar->headers);
> > +
> > +		free(lar);
> > +	}
> > +
> > +	return 0;
> > +}
> 
> How about input validation? It is done here in closelar() but not in
> other functions. It would be nice to be consistent and validate
> everywhere. :)

I think you'll find that input validation happens in all of the API
functions - I think thats sufficient.  The API functions should be
able to verify that everything is good before sending them to the
static helper functions.

> Also suggest
> 
> if (!lar)
>   return -1;
> 
> right at the start of the function rather than indenting lots of code
> that also covers the common case.

Done.  But I'm returning 0 - no LAR is not a failure in this case.

> > +static void *get_header_by_name(struct LAR *lar, const char *name)
> > +{
> > +	struct lar_header *header;
> 
> Why not return struct lar_header * ?

Fixed.

> > +int lfseek(struct LFILE *file, long offset, int whence)
> > +{
> > +	switch(whence) {
> > +	case SEEK_SET:
> > +		file->offset = offset;
> > +		break;
> > +	case SEEK_CUR:
> > +		file->offset =+ offset;
> 
> Does that work? I've only evern seen +=

Typo.  Fixed;

> 
> > +	case SEEK_END:
> > +		file->offset = file->size - offset;
> > +		break;
> 
> Should this not be file->size + offset?

Yeah, but SEEK_END doesn't work anyway, so I changed it to return
-1.  You can't seek past the end of a LAR.

> Great work, but I'm not ready to ack yet.

I'll wait for other comments before reposting a patch.

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.