On Tue, Jun 15, 2010 at 09:32:58AM +0200, Andrew Beekhof wrote:
Looks pretty good.
Do we know the license of the CRC code? That's probably important.
The code doesn't have an explicit license in it, so I've emailed the
author to ask permission to include the code. I'll plead ignorance on
what existing library might already provides a CRC32 function: any
recommendation?
I think the t_datahandler_payload struct can be streamlined a bit -
there's no real need for both request and response fields.
The convention is to put "t_" at the end btw... ie. uint32_t.
Also to use zero-length arrays for payloads.
typedef struct
{
char* request_id;
matahari_message_type_t payload_type;
unsigned long payload_length;
unsigned long payload_crc;
char payload[0];
} matahari_message_t;
I'd also advise adding a MH_ prefix to the members of the
matahari_message_type_t enum.
multiplexer_get_api_function() is probably ok for now, but we might
want to think about using a hash table to speed it up a bit.
I'll incorporate the feedback and send out an updated patch. Also I'll
remove any const char* references as well.
--
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/