configure.in with-pid-file

[Joe Shaw]

Hi,

On Fri, 2004-06-11 at 10:58 +0200, Ikke wrote:
> I worked some more on this. Hal should create its pid file too now.

Thanks for the patch.  A couple of things: can you attach patches
instead of putting them inline?  It makes them easier to save and apply.

Also:

> Index: hald/hald.c
> ===================================================================
> RCS file: /cvs/hal/hal/hald/hald.c,v
> retrieving revision 1.8
> diff -u -b -B -r1.8 hald.c
> --- hald/hald.c 1 Jun 2004 21:10:02 -0000       1.8
> +++ hald/hald.c 11 Jun 2004 08:53:09 -0000
> @@ -225,6 +225,7 @@
>         if (opt_become_daemon) {
>                 int child_pid;
>                 int dev_null_fd;
> +                int pfd;
>  
>                 if (chdir ("/") < 0) {
>                         HAL_ERROR (("Could not chdir to /, errno=%d",
> @@ -239,7 +240,25 @@
>                         break;
>  
>                 case 0:
> -                       /* child */
> +                        pfd = open(HALD_PID_FILE, O_RDWR | O_CREAT,
> 0640);

Mode should probably be O_WRONLY.

I'd consider adding O_TRUNC too, but that might not jive well with the
locking below.  Problem is that if the first time hald runs and the pid
is "23948" you'll have a pid file like:

        23948
        
but the second time it runs and only has a pid of, say, 450, then you
get this:

        450
        8

and any tools that depend on just catting the pid file will fail.

And is there any reason to set the mode to 0644?

> +                        int pfd_valid=1;

This is C99 and we're still using C89, so declare pfd_valid at the top
of the block (with pfd).  Also, as a style thing, please put spaces
before and after the equals sign.

> +
> +                        if (pfd < 0) {
> +                                HAL_ERROR(("Cannot open pidfile"));
> +                                pfd_valid=0;
> +                        }
> +                        if (lockf(pfd, F_TLOCK, 0) < 0) {
> +                                HAL_ERROR(("Cannot get a lock on
> pidfile"));
> +                                pfd_valid=0;
> +                        }
> +
> +                        if (pfd_valid = 1) {

You want == here, not =.

> +                                //Our file descriptor is valid, lets
> write the pid

Please use C89-style comments (/* */)

> +                                char str[10];
> +
> +                                sprintf(str, "%d\n", getpid());

Because I'm a pedant, I'd suggest snprintf() instead.

> +                                write(pfd, str, strlen(str));
> +                        }
>                         dev_null_fd = open ("/dev/null", O_RDWR);
>                         /* ignore if we can't open /dev/null */

Thanks,
Joe


_______________________________________________
hal mailing list
hal at freedesktop.org
http://freedesktop.org/mailman/listinfo/hal