Re: [Netcfg--Add-WPA-support 3/3] Add WPA support

[Glenn Saberton <gsaberton@foomagic.org>]

Reinhard Tartler wrote:
> Glenn Saberton <gsaberton@foomagic.org> writes:
> 
>> Reinhard Tartler wrote:
>>> Glenn Saberton <gsaberton@foomagic.org> writes:
>>>
>>>> +static int start_wpa_daemon(struct debconfclient *client)
>>>> +{
>>>> +    wpa_supplicant_pid = fork();
>>>> +
>>>> +    if (wpa_supplicant_pid == 0) {
>>>> +        fclose(client->out);
>>>> +        if (execlp("wpa_supplicant", "wpa_supplicant", "-i", interface, "-C",
>>>> +                   WPASUPP_CTRL, "-P", WPAPID, "-B", NULL) == -1) {
>>>> +            di_error("could not exec wpasupplicant: %s", strerror(errno));
>>>> +            return 1;
>>>> +        }
>>>> +        else 
>>>> +            return 0;
>>>> +    }
>>>> +    else {
>>>> +        waitpid(wpa_supplicant_pid, NULL, 0);
>>>> +        return 0;
>>>> +    }
>>>> +}
>>> this looks fishy. Are you sure you want to return if the child has
>>> failed to exec?
>>>
>> Can you explain what you mean by looks fishy? Suggestions and
>> improvements are more than welcome, and I still have a few things I want
>> to do yet myself. I'm a bit of a C novice, so I am more than likely to
>> have done a few things the less than perfect way, but I'm not sure what
>> you mean by the above comment. As is, if the exec fails, then we return
>> back, which could really do with an error template as well. I think I
>> still need to do a bit more work on the error handling side of things.
> 
> fork() clones the current process. The exec then tries to replace the
> "child" process with a new one, in this case wpasupplicant. If
> wpasupplicant can be executed and the exec call succeeds, it never
> returns.
> 
> However if exec fails, you generally don't want to continue execution,
> but exit with an error code.
> 
> If done like you did here, you'll have 2 netcfg processes in the
> system. From the first look, it doesn't look that this is intended.
> 
> 
> 
Ah, I see what you mean. I hadn't thought of that. Thanks!

Cheers

Glenn