Interface Patch

[Brian Aker]

Hi!

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) {
                 perror("bind()");
                 /* If we are not at the first element, loop back and  
close all sockets */
                 if (sfd_ptr != sfd_list) {
                     --sfd_ptr;
                     do {
                         close(*sfd_ptr);
                         --sfd_ptr;
                     } while (sfd_ptr != sfd_list);
                 }
                 close(sfd);
                 freeaddrinfo(ai);
                 free(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.

Cheers,
	-Brian

--
_______________________________________________________
Brian "Krow" Aker, brian at tangent.org
Seattle, Washington
http://krow.net/                     <-- Me
http://tangent.org/                <-- Software
http://exploitseattle.com/    <-- Fun
_______________________________________________________
You can't grep a dead tree.