Interface Patch
Brian Aker
brian at tangent.org
Mon Feb 4 08:44:49 UTC 2008
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.
More information about the memcached
mailing list