On (02/24/09 23:59), Peter Memishian wrote:
> write them once :-)  Another missing error case: parsable mode must require
> a field list (for the same reason).

I don't get this one? You mean the case when no template is passed
to ofmt_open()? That should always be an error (even in the !parsable
case) and I am catching it in the new webrev

> nit, it'd be preferable to start all the ofmt errors (except OFMT_SUCCESS,
> which isn't really an error) with OFMT_E to make it clear it's an error
> code.  The libipmp and libdlpi libraries follow the same convention.

done

> In ofmt_open(), I'd find this more logical:
> 
>               err = OFMT_BADFIELDS;
>               if (os->os_badfields == NULL)
>                       os->os_badfields = malloc(...

Ok. done.

> -->          3 file(s)      65792 bytes                   

I'd forgotten about that. :-)

> What is "undefined"?  AFAIK, a property's value is always defined, though
> it may have an empty value.

I was thinking of things like properties that may not be defined for
some link type (we return NOTSUP for these cases). But as you point
out, it's simpler to just have a TRUE/FALSE return, treating all FALSE
returns as "?"- this keeps the API simple for the mainstream callers,
and those that need finer-grained error codes and mnemonics can 
define these internally when setting up the output buffer.

> 
>  > >  * 45-46: No need for these static declarations.
> 
> Oops, I meant to remove this comment as I subsequently re-read the comment
> on line 36 that makes it clear that you're trying to provide the complete
> mini-API for this bit of functionality.  Sorry about that.
> 

anyway, latest (final?) webrevs are available in the usual
places. Both webrev and webrev.meem have been updated.

--Sowmini


Reply via email to