> Here is my patch to increase the error checking within libnetfs.
> I've also included error.h in a couple of files that I noticed had compiler
> warnings. There aren't any functions that return pointers to struct's
> left in libnetfs.
There is one in libdiskfs (diskfs_make_node). Do you want to fix this
too?
> Apparently the copyright wasn't updated in console-run.c during the last
> set of checkins.
>
> Just a reminder, there are outstanding documentation patches for hurd.texi.
Could you write these?
> nfs:
> * nfs.h: Changed lookup_fhandle to return an error_t instead
> of void.
I think you should do:
* nfs.h (lookup_fhandle): Now returns error_t instead of void.
> nfsd:
> * fsys.c: Include error.h, so that the error function is explicitly
> declared.
This should read:
* fsys.c: Include <error.h>.
You need to <>s and there is no need for an explanation in this case.
> Index: ftpfs/fs.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/ftpfs/fs.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 fs.c
> --- ftpfs/fs.c 29 Dec 2001 00:19:39 -0000 1.4
> +++ ftpfs/fs.c 29 Mar 2002 06:12:05 -0000
> @@ -64,10 +64,8 @@ ftpfs_create (char *rmt_path, int fsid,
>
> if (! err)
> {
> - super_root = netfs_make_node (0);
> - if (! super_root)
> - err = ENOMEM;
> - else
> + err = netfs_make_node (0, &super_root);
> + if (! err)
> {
> err = ftpfs_dir_create (new, super_root, rmt_path, &super_root_dir);
> if (! err)
This seems wrong: you ignore netfs_make_node's error case!
> Index: libnetfs/fsys-getroot.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/libnetfs/fsys-getroot.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 fsys-getroot.c
> --- libnetfs/fsys-getroot.c 16 Jun 2001 20:23:29 -0000 1.10
> +++ libnetfs/fsys-getroot.c 29 Mar 2002 06:12:07 -0000
> @@ -127,20 +128,28 @@ netfs_S_fsys_getroot (mach_port_t cntl,
>
> flags &= ~OPENONLY_STATE_MODES;
>
> - newpi = netfs_make_protid (netfs_make_peropen (netfs_root_node, flags,
> - &peropen_context),
> - cred);
> - mach_port_deallocate (mach_task_self (), dotdot);
> -
> - *do_retry = FS_RETRY_NORMAL;
> - *retry_port = ports_get_right (newpi);
> - *retry_port_type = MACH_MSG_TYPE_MAKE_SEND;
> - retry_name[0] = '\0';
> - ports_port_deref (newpi);
> -
> + err = netfs_make_peropen (netfs_root_node, flags, &peropen_context, &newpo);
> + if (! err)
> + {
> + err = netfs_make_protid (newpo, cred, &newpi);
> + if (err)
> + netfs_release_peropen (newpo);
> + }
> +
> + if (! err)
> + {
> + mach_port_deallocate (mach_task_self (), dotdot);
I think this needs to happen no matter what (i.e. error or not).
> +
> + *do_retry = FS_RETRY_NORMAL;
> + *retry_port = ports_get_right (newpi);
> + *retry_port_type = MACH_MSG_TYPE_MAKE_SEND;
> + retry_name[0] = '\0';
> + ports_port_deref (newpi);
So does this.
> + }
> + else
> out:
> - if (err)
> iohelp_free_iouser (cred);
> +
> mutex_unlock (&netfs_root_node->lock);
> return err;
> }
> Index: libnetfs/io-duplicate.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/libnetfs/io-duplicate.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 io-duplicate.c
> --- libnetfs/io-duplicate.c 16 Jun 2001 20:23:29 -0000 1.3
> +++ libnetfs/io-duplicate.c 29 Mar 2002 06:12:07 -0000
> @@ -35,10 +35,15 @@ netfs_S_io_duplicate (struct protid *use
> return err;
>
> mutex_lock (&user->po->np->lock);
> - newpi = netfs_make_protid (user->po, clone);
> - *newport = ports_get_right (newpi);
> - mutex_unlock (&user->po->np->lock);
> - *newporttp = MACH_MSG_TYPE_MAKE_SEND;
> - ports_port_deref (newpi);
> - return 0;
> + err = netfs_make_protid (user->po, clone, &newpi);
> + if (! err)
> + {
> + *newport = ports_get_right (newpi);
> + mutex_unlock (&user->po->np->lock);
You need to always unlock USER->po->np->lock. Not just on success.
> + *newporttp = MACH_MSG_TYPE_MAKE_SEND;
> + ports_port_deref (newpi);
Same idea here.
> + return 0;
> + }
> + else
> + return err;
> }
> Index: libnetfs/io-reauthenticate.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/libnetfs/io-reauthenticate.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 io-reauthenticate.c
> --- libnetfs/io-reauthenticate.c 16 Jun 2001 20:23:29 -0000 1.10
> +++ libnetfs/io-reauthenticate.c 29 Mar 2002 06:12:07 -0000
> @@ -1,5 +1,5 @@
> /*
> - Copyright (C) 1995,96,2000,01 Free Software Foundation, Inc.
> + Copyright (C) 1995,96,2000,01,02 Free Software Foundation, Inc.
> Written by Michael I. Bushnell, p/BSG.
>
> This file is part of the GNU Hurd.
> @@ -32,22 +32,28 @@ netfs_S_io_reauthenticate (struct protid
> return EOPNOTSUPP;
>
> mutex_lock (&user->po->np->lock);
> - newpi = netfs_make_protid (user->po, 0);
> + err = netfs_make_protid (user->po, 0, &newpi);
>
> - newright = ports_get_send_right (newpi);
> - assert (newright != MACH_PORT_NULL);
> + if (! err)
> + {
>
> - err = iohelp_reauth (&newpi->user, netfs_auth_server_port, rend_port,
> - newright, 1);
> -
> - mach_port_deallocate (mach_task_self (), rend_port);
> - mach_port_deallocate (mach_task_self (), newright);
> -
> - mach_port_move_member (mach_task_self (), newpi->pi.port_right,
> - netfs_port_bucket->portset);
> -
> - mutex_unlock (&user->po->np->lock);
> - ports_port_deref (newpi);
> -
> - return err;
> + newright = ports_get_send_right (newpi);
> + assert (newright != MACH_PORT_NULL);
> +
> + err = iohelp_reauth (&newpi->user, netfs_auth_server_port, rend_port,
> + newright, 1);
> +
> + mach_port_deallocate (mach_task_self (), rend_port);
> + mach_port_deallocate (mach_task_self (), newright);
> +
> + mach_port_move_member (mach_task_self (), newpi->pi.port_right,
> + netfs_port_bucket->portset);
> +
> + mutex_unlock (&user->po->np->lock);
This needs to be called in both cases, i.e. error and success.
> + ports_port_deref (newpi);
As does this.
> +
> + return err;
> + }
> + else
> + return err;
> }
> Index: libnetfs/io-restrict-auth.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/libnetfs/io-restrict-auth.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 io-restrict-auth.c
> --- libnetfs/io-restrict-auth.c 16 Jun 2001 20:37:39 -0000 1.4
> +++ libnetfs/io-restrict-auth.c 29 Mar 2002 06:12:07 -0000
> @@ -1,5 +1,5 @@
> /*
> - Copyright (C) 1995,96,2001 Free Software Foundation, Inc.
> + Copyright (C) 1995,96,2001,2002 Free Software Foundation, Inc.
> Written by Michael I. Bushnell, p/BSG.
>
> This file is part of the GNU Hurd.
> @@ -96,21 +96,17 @@ netfs_S_io_restrict_auth (struct protid
> }
>
> mutex_lock (&user->po->np->lock);
> - newpi = netfs_make_protid (user->po, new_user);
> - if (newpi)
> + err = netfs_make_protid (user->po, new_user, &newpi);
> + if (! err)
> {
> *newport = ports_get_right (newpi);
> - mutex_unlock (&user->po->np->lock);
> *newporttype = MACH_MSG_TYPE_MAKE_SEND;
> + ports_port_deref (newpi);
> }
> - else
> - {
> - mutex_unlock (&user->po->np->lock);
> - iohelp_free_iouser (new_user);
> - err = ENOMEM;
> - }
> -
> - ports_port_deref (newpi);
> + mutex_unlock (&user->po->np->lock);
> +
> + iohelp_free_iouser (new_user);
This change looks wrong. new_user should not be freed in the success
case. Also, ports_port_deref needs to happen after the unlock.
> return err;
> +
> }
>
> Index: libnetfs/netfs.h
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/libnetfs/netfs.h,v
> retrieving revision 1.29
> diff -u -p -r1.29 netfs.h
> --- libnetfs/netfs.h 30 Jan 2001 00:50:25 -0000 1.29
> +++ libnetfs/netfs.h 29 Mar 2002 06:12:08 -0000
> @@ -263,11 +263,6 @@ error_t netfs_attempt_write (struct ious
> error_t netfs_report_access (struct iouser *cred, struct node *np,
> int *types);
>
> -/* The user must define this function. Create a new user from the
> - specified UID and GID arrays. */
> -struct iouser *netfs_make_user (uid_t *uids, int nuids,
> - uid_t *gids, int ngids);
> -
> /* The user must define this function. Node NP has no more references;
> free all its associated storage. */
> void netfs_node_norefs (struct node *np);
> @@ -341,9 +336,9 @@ extern int netfs_maxsymlinks;
> /* Definitions provided by netfs. */
>
> /* Given a netnode created by the user program, wraps it in a node
> - structure. The new node is not locked and has a single reference.
> - If an error occurs, NULL is returned. */
> -struct node *netfs_make_node (struct netnode *);
> + structure, *nnp. The new node is not locked and has a single
> reference.
Local variables and arguments must be in caps. I.e. NNP.
> + If an error occurs, the error value is returned. */
> +error_t netfs_make_node (struct netnode *nn, struct node **nnp);
> /* Whenever node->references is to be touched, this lock must be
> held. Cf. netfs_nrele, netfs_nput, netfs_nref and netfs_drop_node. */
> @@ -363,15 +358,16 @@ mach_port_t netfs_startup (mach_port_t b
> void netfs_server_loop (void);
>
> /* Creates a new credential from USER which can be NULL on the peropen
> - PO. Returns NULL and sets errno on error. */
> -struct protid *netfs_make_protid (struct peropen *po, struct iouser *user);
> + PO. Returns the error value. */
> +error_t netfs_make_protid (struct peropen *po, struct iouser *user,
> + struct protid **pi);
>
> -/* Create and return a new peropen structure on node NP with open
> +/* Create a new peropen structure, *ppo, on node NP with open
Likewise for PPO.
> flags FLAGS. The initial values for the root_parent, shadow_root,
> and shadow_root_parent fields are copied from CONTEXT if it's
> - non-zero, otherwise zeroed. */
> -struct peropen *netfs_make_peropen (struct node *, int,
> - struct peropen *context);
> + non-zero, otherwise zeroed. Returns the error value. */
> +error_t netfs_make_peropen (struct node *np, int, struct peropen *context,
> + struct peropen **ppo);
>
> /* Add a reference to node NP. Unless you already hold a reference,
> NP must be locked. */
> Index: trans/firmlink.c
> ===================================================================
> RCS file: /cvsroot/hurd/hurd/trans/firmlink.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 firmlink.c
> --- trans/firmlink.c 26 Feb 2001 04:16:01 -0000 1.12
> +++ trans/firmlink.c 29 Mar 2002 06:12:17 -0000
> @@ -1,8 +1,9 @@
> /* A translator for `firmlinks'
>
> - Copyright (C) 1997, 1998, 1999, 2001 Free Software Foundation, Inc.
> + Copyright (C) 1997, 1998, 1999, 2001, 2002 Free Software Foundation, Inc.
>
> Written by Miles Bader <[EMAIL PROTECTED]>
> + Extended by James A. Morrison <[EMAIL PROTECTED]>
>
> This program is free software; you can redistribute it and/or
> modify it under the terms of the GNU General Public License as
> @@ -26,42 +27,91 @@
> #include <fcntl.h>
> #include <argp.h>
> #include <error.h>
> +#include <time.h>
> #include <sys/mman.h>
>
> #include <hurd/trivfs.h>
>
> #include <version.h>
>
> +#define TIME_MODE 0
> +#define RANDOM_MODE 1
> +#define SEQUENCE_MODE 2
> +
> const char *argp_program_version = STANDARD_HURD_VERSION (firmlink);
>
> static const struct argp_option options[] =
> {
> + {"randomized", 'r', NULL, 0, "Randomize selection of files, default" },
> + {"sequencial", 's', NULL, 0, "Choose files sequencially" },
> + {"format",'f',"DATE FORMAT", 0,
> + "Create files named file1/file2.DATE FORMAT"},
> { 0 }
> };
>
> -static const char args_doc[] = "TARGET";
> -static const char doc[] = "A translator for firmlinks."
> +static const char args_doc[] = "[file1 file2 ...]";
> +static const char doc[] = "A translator for multiple firmlinks."
> "\vA firmlink is sort of half-way between a symbolic link and a hard link:"
> "\n"
> "\nLike a symbolic link, it is `by name', and contains no actual reference to"
> " the target. However, the lookup returns a node which will redirect parent"
> " lookups so that attempts to find the cwd that go through the link will"
> " reflect the link name, not the target name. The target referenced by the"
> -" firmlink is looked up in the namespace of the translator, not the client.";
> +" firmlink is looked up in the namespace of the translator, not the client."
> +" A multiple firmlink can point to more than one file, and can also create"
> +" files based on dates.";
>
> /* Link parameters. */
> -static char *target = 0; /* What we translate too. */
> +
> +struct arg_struct
> +{
> + char **args;
> + char *date_format;
> + int type;
> + int flags;
> + unsigned long lastused;
> + unsigned long size;
> +};
> +
> +struct arg_struct config;
>
> /* Parse a single option/argument. */
> static error_t
> parse_opt (int key, char *arg, struct argp_state *state)
> {
> - if (key == ARGP_KEY_ARG && state->arg_num == 0)
> - target = arg;
> - else if (key == ARGP_KEY_ARG || key == ARGP_KEY_NO_ARGS)
> - argp_usage (state);
> - else
> - return ARGP_ERR_UNKNOWN;
> + struct arg_struct *arguments = state->input;
> +
> + switch (key)
> + {
> + case 'r':
> + arguments->type = RANDOM_MODE;
> + arguments->flags = O_CREAT;
> + break;
> + case 's':
> + arguments->type = SEQUENCE_MODE;
> + arguments->flags = O_CREAT;
> + arguments->lastused = 0;
> + break;
> + case 'f':
> + arguments->type = TIME_MODE;
> + arguments->date_format = arg;
> + arguments->flags = 0;
> + break;
> + case ARGP_KEY_ARG:
> + arguments->args[arguments->size] = arg;
> + arguments->size++;
> + break;
> + case ARGP_KEY_END:
> + if ( arguments->type == TIME_MODE && arguments->size != 2 )
> + argp_usage (state);
> + break;
> + case ARGP_KEY_NO_ARGS:
> + argp_usage (state);
> + break;
> + default:
> + return ARGP_ERR_UNKNOWN;
> + }
> +
Please put this in a seperate different patch. Not to mention, there
are no change log entries indicating these changes.
> return 0;
> }
>
> @@ -74,8 +124,16 @@ main (int argc, char **argv)
> mach_port_t bootstrap;
> struct trivfs_control *fsys;
>
> + config.size = 0;
> + config.type = RANDOM_MODE;
> + config.args = (char**) malloc (argc);
> + if (! config.args)
> + error (ENOMEM, 1, "Starting up");
> +
> /* Parse our options... */
> - argp_parse (&argp, argc, argv, 0, 0, 0);
> + argp_parse (&argp, argc, argv, 0, 0, &config);
> + if ( config.type == RANDOM_MODE )
> + srand (time (NULL) );
Check these parentheseses.
> task_get_bootstrap_port (mach_task_self (), &bootstrap);
> if (bootstrap == MACH_PORT_NULL)
> @@ -102,7 +160,26 @@ firmlink (mach_port_t parent, const char
> {
> error_t err;
> file_t authed_link;
> - file_t target = file_name_lookup (target_name, flags & ~O_CREAT, 0);
> + file_t target = file_name_lookup (target_name, flags & ~config.flags, 0);
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd
