Interface Patch

Brian Aker brian at
Mon Feb 4 08:44:49 UTC 2008


On Feb 3, 2008, at 11:37 PM, Tomash Brechko wrote:

> event_add() is not a system call, perror() may or may not return
> something meaningful, depending on whether the failure happened in a
> syscall executed from event_add(), or in the event_add()'s own logic.

I found that bit when looking through the libevent mailing lists. The  
previous call that would generate the error will be a system call.

> Likewise, malloc() and friends are not system calls, and do not set  
> errno.

Good point. Though if you look through the code that is the  
convention :)
I will just fix all of them in memached.c

> On a larger scale, you are storing the array size now along with the
> array.  Instead of using 'success' counter in the binding code, you
> could store in *count the number of _initialized_ sockets.  Then the
> whole thing will be
>  - malloc() the array
>  - initialize continuous number of elements from the start, set
>    *count to the number of initialized elements, leaving the rest
>    uninitialized.
> With this, no initialization and later tests against -1 will be
> required, you will always know that first *count elems are valid.

That seemed a bit of a hack to do it that way (aka keeping first  
element for size). The count bit can go away in the future if the loop  
for socket creation/event creation is factored into one. You still  
need both success and count though. You could turn the entire object ino

>> +              for (sfd_ptr= sfd_list, x = 0; x < *count; sfd_ptr+ 
>> +) {
>> +                  if (*sfd_ptr > -1 )
>> +                      close(sfd);
>> +              }
> Copy-pastes are evil ;).

Well, the solution for the above is to toss into a goto, to an error  
block. But that freaks people out :)

For instance this:
             if (errno != EADDRINUSE) {
                 /* If we are not at the first element, loop back and  
close all sockets */
                 if (sfd_ptr != sfd_list) {
                     do {
                     } while (sfd_ptr != sfd_list);
                 return NULL;

bind() and listen() have the same error control. It would be possible  
to just add a goto error: and then in the error label have just one  
section of code.

Thanks for catching the infinite loop in the error block.


Brian "Krow" Aker, brian at
Seattle, Washington                     <-- Me                <-- Software    <-- Fun
You can't grep a dead tree.

More information about the memcached mailing list