Re: [Libevent-users] evrpc_status structure.

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Sun, Feb 3, 2013 at 10:58 AM, fabien pichot <pichot.fabien@xxxxxxxxx> wrote:
> Hello,
>
> I didn't lurk that ML that much, so I'm not really sure about the way to send
> patches. But anyway, attached is a patch that makes the error codes public and
> add two functions to get the error and the http request from the evrpc_status
> structure.

Hi, Fabien!  Attachments are fine.  Here's a quick review:

1) Turning the values from #define into an enum will create problems
for users of C++ compilers that like to give warnings about int/enum
conversions.  Probably not a great idea, since it would break existing
code.

2) The names for these functions should probably be
evrpc_status_get_FIELD, rather than evrpc_status_FIELD.  (yes, some
older accessor function names in libevent don't follow the
TYPE_get_FIELD convention -- but we're trying not to do that with new
function names.)

3) The new functions and values need documentation in the header.

4) There's no unit test for the evhttp_request accessor.

5) The formatting doesn't match our conventions: we try to put a
newline between return types and function names only when defining a
function, not when declaring it.

If you're able to fix these in a new patch, that would be excellent.

best wishes,
-- 
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.