[PATCH v2] Allow PID file to be written in foreground mode

[Jouni Malinen]

On Wed, Dec 12, 2012 at 12:28:10PM +0100, Pontus Fuchs wrote:
> I wanted hostapd to write a PID file in foreground mode but I found
> that it couldn't. It will silently ignore the -P option when -B is
> not specified on the command line.
> 
> Fix this by moving the PID file writing from os_daemonize to a new
> function.

What's the use case for this? The previous pair of os_daemonize and
os_daemonize_terminate was matching pretty nicely while this this
combination of three functions and especially the name of
os_daemonize_terminate in this case is not as clean.

> V2:
> * Fix a typo in commit msg.
> * Fix the change to wpa_cli.c which accidently got chewed when moving
>   the patch to a newer tree. Compilation was still fine so I didn't
>   see it.

Hmm.. There is still something strange with wpa_cli.c.


> diff --git a/src/utils/os.h b/src/utils/os.h
> @@ -77,10 +77,9 @@ int os_gmtime(os_time_t t, struct os_tm *tm);
> -int os_daemonize(const char *pid_file);
> +int os_daemonize();

That should be os_daemonize(void) (and same for the eloop*.c files).

> +int os_write_pid_file(const char *pid_file)
> +{
>  	if (pid_file) {
>  		FILE *f = fopen(pid_file, "w");
>  		if (f) {
>  			fprintf(f, "%u\n", getpid());
>  			fclose(f);
> +			return 0;
>  		}
>  	}
> -
> -	return -0;
> +	return -1;

Should this really return -1 (error?) if pid_file is not set. The
changes you made for the callers seem to be ignoring the return value
completely, so this could as well be a void function..

> diff --git a/wpa_supplicant/wpa_cli.c b/wpa_supplicant/wpa_cli.c
> @@ -3482,8 +3482,10 @@ int main(int argc, char *argv[])
> -		if (daemonize && os_daemonize(pid_file))
> +		if (daemonize && os_daemonize()) {
>  			return -1;
> +			os_write_pid_file(pid_file);
> +		}

??

> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -2400,10 +2400,10 @@ int wpa_supplicant_driver_init(struct wpa_supplicant *wpa_s)
> -static int wpa_supplicant_daemon(const char *pid_file)
> +static int wpa_supplicant_daemon()

(void)

Though, with what remains in the function, you might as well remove this
unnecessary wrapper completely.

-- 
Jouni Malinen                                            PGP id EFC895FA