> 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
I mean that "ipmpstat -P" is an error without also specifying "-o". This is similar to "-P" with "-o all" being an erorr, but should be a distinct error because we'd like a distinct error message. > > 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). My expectation was that the framework would be smart enough to not invoke a callback that doesn't apply. Otherwise, I'm not sure how the callback has enough contextual knowledge to distinguish an expected failure (e.g., unsupported) from an unexpected failure (e.g., unable to get result). Did you have something in mind here? > 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. I'm not sure I understand how the finer-grained error codes would work. Can you provide an example? I consider it an architectural mistake to use the string buffer to represent states that are not actually values, as it means there is then no way to represent those values. So if you really think this case is likely, having an enumeration makes sense. In any case, this API is consolidation-private for now so we can always change this (though it is tedious to update all the callers). > anyway, latest (final?) webrevs are available in the usual Given that I've only reviewed two files (ofmt.c and ipmpstat.c) and I've seen very few comments on the other files, I'm surprised this is being considered a possible final webrev. > places. Both webrev and webrev.meem have been updated. -- meem
