Hi Aneurin,

> Nobody has any comments about this?

Well, I held off thinking I'd give Alexandre a chance to chime in.  As
it's a large patch, I knew it could take a while.  But since he hasn't..

My advice isn't going to be as specific as it has been in the past. 
You've obviously done a lot on this, and it looks like useful and
well-done work.  A couple tips to improve its chances then:

1. Write some test cases.  For example, in the code you comment:
    /* Further size specifiers are swallowed; this isn't specified, but
was determined from testing.
     * Thus "ll", for example, does not mean "long long" (I64 should be
used instead), but "long"
     */
Writing a test case that checks this is better than commenting the code,
because it really ensures future changes won't break it.  Generally, test
cases help demonstrate the correctness of the patch (and the incorrectness
of what's already there.)  Once you're written one, they get pretty simple
to do.

2. I'm troubled a bit by the memory allocation.  It seems as though it
should be possible to output characters or formatted output one at a time,
keeping track of where you are in the format string and how many
characters you are allowed to output.  I realize it's a bit difficult
since you may be outputting to either a file pointer or to a buffer, but
modulo that complication you should be able to avoid allocating temporary
buffers.  I know, somewhere in the FAQ it says something about correctness
before efficiency, but this bit of inefficiency seems pretty avoidable ;)

Hope that helps,
--Juan

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

Reply via email to