Alex Riesen wrote:
>Stephen R. van den Berg, Tue, Aug 12, 2008 23:25:35 +0200:
>> + struct child*newborn;
>You may want to reformat the patch using tabs instead of spaces.
Tried to do it right, but apparently missed a few spots, thanks.
>> + newborn = xmalloc(sizeof *newborn);
>The custom here is to use "sizeof(type)". Brackets and typename.
Well, as for the parens, current git source shows a ratio of 26 : 887
in your favour; apparently not everyone agrees, but most do.
As for the typename, that rather uncommon, in most cases actual
variables are used (with good reason).
>> + if (newborn) {
>> + struct child**cradle,*blanket;
>"struct child **cradle, *blanket;" (the spaces before asterisks)
I'll be more careful.
>> + memcpy(memset(&newborn->address, 0, sizeof newborn->address),
>> + addr, addrlen);
>Aren't separate calls easier to read (and type)?
Possibly, yes.
> memset(&newborn->address, 0, sizeof(newborn->address));
> memcpy(&newborn->address, addr, addrlen);
But it results in more machinecode in most cases. Sorry, is my builtin
micro-optimiser at work.
>> -static void kill_some_children(int signo, unsigned start, unsigned stop)
>> +static void kill_some_child(int signo)
>> {
>> - start %= MAX_CHILDREN;
>> - stop %= MAX_CHILDREN;
>> - while (start != stop) {
>> - if (!(start & 3))
>> - kill(live_child[start].pid, signo);
>> - start = (start + 1) % MAX_CHILDREN;
>> + const struct child *blanket;
>> +
>> + if ((blanket = firstborn)) {
> if (firstborn) {
> const struct child *blanket = firstborn;
>You don't even use blanket outside of the "if".
Good thinking. I restructured the code a few times, previously this
wasn't possible.
>> static void check_dead_children(void)
>> {
>> + loginfo("[%d] Disconnected%s", (int)pid, dead);
>BTW, why do you need that pid_t->int cast?
Because pid_t might be wider than int (unlikely, but possible),
and then this call might crash the program.
>> @@ -1105,6 +1026,10 @@ int main(int argc, char **argv)
>> init_timeout = atoi(arg+15);
>> continue;
>> + if (!prefixcmp(arg, "--max-connections=")) {
>> + max_connections = atoi(arg+18);
>An error checking wouldn't go amiss. And it can't be done with atoi
>(consider strtol).
I merely copied the other argument parsing methods, didn't want to
improve this, just functionally equivalent (will do fine here, IMO).
--
Sincerely,
Stephen R. van den Berg.
Father's Day Special at the local clinic -- Vasectomy!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
