[darcs-devel] darcs patch: Improve error diagnostics in compat.c's careful_atomic...

Ian Lynagh igloo at earth.li
Wed Apr 20 18:58:47 PDT 2005


Hi Ralph,

On Mon, Apr 18, 2005 at 09:07:00AM -0400, Ralph Corderoy wrote:

> [Improve error diagnostics in compat.c's careful_atomic_create().
> Ralph Corderoy <ralph at inputplus.co.uk>**20050417225410
>  Add fprintf()s where there were none.  Ensure useful variables are
>  printed in them.  Add error checking on gettimeofday().  Remove needless
>  string termination immediately before snprintf().  Check return value
>  against -1 since that's the defined interface and more idiomatic/simpler
>  to parse than < 0.
> ] {
> hunk ./compat.c 85

           rc = gethostname(hostname, 65);

> -        if(rc < 0 || rc >= 65) {
> -            fprintf(stderr, "Error reading hostname when locking.\n");
> +        if (rc == -1 || rc >= 65) {
> +            fprintf(stderr, "careful_atomic_create: gethostname "
> +                "failed: %d %d\n", rc, errno);

Hmm, my manpage says

       These  functions  are  used to access or to change the host name of the
       current processor.  The gethostname() function returns a NUL‐terminated
       hostname  (set  earlier  by sethostname()) in the array name that has a
       length of len bytes.  In case the NUL‐terminated hostname does not fit,
       no  error is returned, but the hostname is truncated. It is unspecified
       whether the truncated hostname will be NUL‐terminated.

[...]

    BUGS
       For many Linux kernel / libc combinations gethostname  will  return  an
       error instead of returning a truncated hostname.

I don't see a problem with checking for >= 65 if that matches what real
implementations return, but shouldn't we also be setting hostname[64] to
'\0'? Well, in fact we then set hostname[15] to '\0', but only after
doing a strchr on it.


       rc = snprintf(...)

> hunk ./compat.c 119
> -    if(rc < 0 || rc >= FILENAME_SIZE) {
> -        fprintf(stderr, "Error writing to lock filename (%d)\n", 
> -                rc < 0 ? errno : 0);
> -        goto fail2;
> +    if(rc == -1 || rc >= FILENAME_SIZE) {
> +        fprintf(stderr, "careful_atomic_create: snprintf failed: %d\n",
> +            rc);
> +        goto fail_post_filename;

My manpage says

   Return value
      [...]
      If an output error is encountered, a negative value is returned.


Thanks
Ian





More information about the darcs-devel mailing list