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