[
Lists Home |
Date Index |
Thread Index
]
Comments I got by email I'm tossing in here so that everything I need
to look at is in one place. These are from Mark Lentczner
I have some comments on your API for genx:
1. Libraries shouldn't ever call abort() or exit() - it limits greatly
how a user of the library structures their own code. A slightly
different error handling strategy would make things a bit easier for
users: Once a genx call generates an error for a given genxWriter,
then all future calls with the same genxWriter immediately return the
same error. This allows a caller to code several calls to genx in a
row, and only check the final result. While you may prefer that people
check after every call, there seems no reason to impose this on users.
2. genxSetError() is fine, but you should allow the user's routine to
return rather than call abort(). Many uses of genx may be unable to
support calls to exit() or abort() (an interactive tool doesn't want to
crash just because some sub-task didn't work.) Just return the error
as normal if the supplied error function returns.
3. There is no way to set a custom free operation to match the custom
allocator. Consider a single routine to set them both (since they
always need to be set in pairs):
genxSetAllocator(genxWriter w,
void * (*alloc)(void * userdata, int bytes),
void (*free)(void * userdata, void * ptr));
4. Throughout the genx library, calls that take text tend to return an
error if the text contains something that isn't allowed at that point
in the XML document. For example, genxComment() fails if the string
contains "--". The essentially means that any user of genx has to
check the strings before genx does, since genx will fail, aborting the
whole document if the string doesn't pass. For the intended audience
of developers, I can see two alternatives that might be more friendly:
4a. Don't return a 'hard', non-recoverable error. Instead, return a
'soft' error that basically says, the string isn't usable in this
context, nothing was done, and you can continue. For example: An
application could easily take the invoking command line and dump it
into an XML comment. In this case, the application probably doesn't
want to do all the work to clear out '--' from the command line. The
solution of just not including the comment is certainly light weight,
the application can check the error if it wants, and if the comment is
omitted, so be it.
4b. Support escaping variants of all such calls. In this case, the
strings will be properly, or 'reasonably' escaped. I realize that such
escaping might alter the document semantics, but often it is okay.
Certainly the comment example above, there is less harm if the users'
command line with '--' is replaced with '- - '.
5. genxStartStartTag()'s prefix handling is unclear. How do I set the
default namespace? By passing "" to prefix? If I pass NULL, then
"genx will create a prefix" - but how do I ever know what prefix it is?
6. The family of genxStartStartTag() calls might be refactored
somewhat: I find that parallel arrays in C tend to give rise to errors,
especially when statically created by hand in the code. Perhaps these
two forms would be better:
int genxStartTag(genxWriter w, const utf8Byte * type, ...)
where ... is one or more sequences of:
GENX_NS_DECL, const utf8Byte * namespace, const utf8Byte *
prefix
GENX_ATTR_DECL, const utf8Byte * name, const utf8Byte * value
GENX_NS_ATTRY_DECL, const utf8Byte * attrPrefix, const utf8 *
name, const utf8Byte * value
GENX_DONE
int genxStartTag(genxWriter w, const utf8Byte * type, const utf8Byte *
* data)
where data points to an array like the above
#define GENX_NS_DECL ((const utf8Byte *)1);
#define GENX_ATTR_DECL ((const utf8Byte *)2);
...etc...
7. It isn't clear if genxText() and genxCharacter() perform entity
replacement for &, <, > and perhaps ". If not, they should have
variants or an options to do so (see 4. above)
8. You might consider adding genxContent(), that dumps content directly
into the document at that point, no checks and no escaping. This would
be used when the calling application has a fragment of, say, XHTML,
that they want to dump in at some point. In a perfect world, genx
would check the structure of such content for well-formedness, but I
suspect you aren't going to put that in.
In general, it looks fine. But I think the issues of escaping is the
biggest problem right now. For the audience this library is intended,
writing out XML should be an almost thoughtless process - which means
that they are going to want escaping to be done for them.
- Mark
Mark Lentczner
http://www.ozonehouse.com/mark/
|
