"Alfred M. Szmidt" <[EMAIL PROTECTED]> wrote:
> [I'm keeping bug-hurd in the CC since maybe someone on that list has
> something to comment.]
>
> Ping.
Thanks for the patch and the prod :-)
Please mention that this option is Hurd-specific
in both --help output and in coreutils.texi.
...
> There is two small problems, the first one is argz.h. Adding a special check
> for it in configure.ac and then doing an ifdef seems abit over kill
> for one function call. Right now it just checks for hurd.h, and
Can you find a better way to do this?
Maybe something relating to support for translators?
Like the existence of this function: file_get_translator.
> assumes that argz.h exists (which will work since all GNU/Hurd systems
> run glibc). If the current solution is not liked then I'll change it
> and send another patch (or the person commiting the change can do it).
>
> The second problem is that I haven't tested if this compiles on
> GNU/Linux, could someone do that for me and report back?
Please document the following
> --translator on GNU/Linux should be a no-op.
...
> 2003-08-16 Alfred M. Szmidt <[EMAIL PROTECTED]>
>
> Add support for new ls option, --translator, for GNU/Hurd.
>
> * doc/coreutils.texi (ls invocation), NEWS: Document this.
> * configure.ac: Check for <hurd.h>.
> * src/ls.c [HAVE_HURD_H]: Include <hurd.h> and <argz.h>.
> (struct fileinfo) [HAVE_HURD_H]: New members trans_name,
> trans_fsid and trans_mode.
> (print_translator): New variable.
> (TRANSLATOR_OPTION): New enum value.
> (long_options, decode_switches, gobble_file)
> (print_long_format, usage): Support --translator.
...
> diff -upr /src-cvs/coreutils/src/ls.c /home/ams/src/coreutils/src/ls.c
> - --- /src-cvs/coreutils/src/ls.c 2003-07-27 13:41:33.000000000 +0200
> +++ /home/ams/src/coreutils/src/ls.c 2003-08-16 21:47:20.000000000 +0200
> @@ -52,6 +52,11 @@
> # include <sys/ptem.h>
> #endif
>
> +#if HAVE_HURD_H
> +# include <hurd.h>
> +# include <argz.h>
> +#endif
> +
> #include <stdio.h>
> #include <assert.h>
> #include <setjmp.h>
> @@ -204,6 +209,18 @@ struct fileinfo
> /* For symbolic link, name of the file linked to, otherwise zero. */
> char *linkname;
>
> +#if HAVE_HURD_H
> + /* The translator that is attached to the node. */
Is this member guaranteed to be set before used?
Could be. I haven't actually applied the patch, and as such
haven't checked carefully enough.
> + char *trans_name;
> +
> + /* The fsid for the active translator. */
> + int trans_fsid;
> +
> + /* If 1 then we have a translator attached and/or running on the node,
> + otherwise 0. */
This member is set but not used.
> + int trans_mode;
> +#endif
> +
...
> @@ -2435,6 +2462,51 @@ gobble_file (const char *name, enum file
> free (linkpath);
> }
>
> +#if HAVE_HURD_H
> + if ((files[files_index].stat.st_mode & S_ITRANS) && print_translator)
> + {
> + int trans_fd;
> + file_t trans_port;
> + struct stat trans_stat;
> +
> + files[files_index].trans_fsid = files[files_index].stat.st_fsid;
> +
> + /* Get the underlying node */
With the following, when the open fails (returning -1), the
fstat will be called unnecessarily:
> + trans_fd = open (path, O_NOTRANS);
> + if ((trans_fd && fstat (trans_fd, &trans_stat)) < 0)
How about this instead:
if ((0 <= trans_fd && fstat (trans_fd, &trans_stat)) < 0)
> + {
> + error (0, errno, "%s", quotearg_colon (path));
Please use a string saying what has failed, rather than just `"%s"'.
E.g.,
error (0, errno, _("failed to get attributes of %s"), quote (path));
> + close (trans_fd);
> + exit_status = 1;
> + return 0;
> + }
> +
> + trans_port = getdport (trans_fd);
> + close (trans_fd);
> +
> + if (trans_stat.st_mode & S_IPTRANS)
> + {
> + char buf[1024], *trans = buf;
Is this 1024-byte buffer guaranteed to be large enough?
Is there some symbolic constant you can use instead of the literal?
> + int trans_len = sizeof (buf);
> +
> + if (file_get_translator (trans_port, &trans, &trans_len))
> + {
> + mach_port_deallocate (mach_task_self(), trans_port);
> + error (0, errno, "%s", quotearg_colon (path));
Same as above. e.g., _("failed to get translator for %s")
> + exit_status = 1;
> + return 0;
> + }
> +
> + argz_stringify (trans, trans_len, ' ');
> +
> + files[files_index].trans_name = strdup(trans);
What if strdup fails?
Either handle it or use xstrdup.
> + files[files_index].trans_mode = 1;
> +
> + mach_port_deallocate (mach_task_self(), trans_port);
...
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd
