xfs
[Top] [All Lists]

Re: REVIEW: xfs_reno #2

To: Barry Naujok <bnaujok@xxxxxxx>
Subject: Re: REVIEW: xfs_reno #2
From: Ruben Porras <ruben.porras@xxxxxxxxxxx>
Date: Thu, 06 Mar 2008 17:10:35 +0100
Cc: "xfs@xxxxxxxxxxx" <xfs@xxxxxxxxxxx>
In-reply-to: <op.tznnweh23jf8g2@pc-bnaujok.melbourne.sgi.com>
References: <op.tznnweh23jf8g2@pc-bnaujok.melbourne.sgi.com>
Sender: xfs-bounce@xxxxxxxxxxx
Am Donnerstag, den 04.10.2007, 14:25 +1000 schrieb Barry Naujok:
> A couple changes from the first xfs_reno:
> 
>   - Major one is that symlinks are now supported, but only
>     owner, group and extended attributes are copied for them
>     (not times or inode attributes).
> 
>   - Man page!
> 
> 
> To make this better, ideally we need some form of
> "swap inodes" function in the kernel, where the entire
> contents of the inode themselves are swapped. This form
> can handle any inode and without any of the dir/file/attr/etc
> copy/swap mechanisms we have in xfs_reno.
> 
> Barry.

+static int
+process_slink(
+       bignode_t       *node)
+{
+       int             i = 0;
+       int             rval = 0;
+       struct stat64   st;
+       char            *srcname = NULL;
+       char            *pname = NULL;
+       char            target[PATH_MAX] = "";
+       char            linkbuf[PATH_MAX];
+
+       SET_PHASE(SLINK_PHASE);
+
+       dump_node("symlink", node);
+
+       cur_node = node;
+       srcname = node->paths[0];
+
+       if (lstat64(srcname, &st) < 0) {
+               if (errno != ENOENT) {
+                       err_stat(srcname);
+                       global_rval |= 2;
+               }
+               goto quit;
+       }


+       if (st.st_ino <= XFS_MAXINUMBER_32 && !force_all)
+               /* this file has changed, and no longer needs processing */
+               goto quit;

This check need to go out, the same in functions process_dir and
process_file.

+       rval = 1;
+
+       i = readlink(srcname, linkbuf, sizeof(linkbuf) - 1);
+       if (i < 0) {
+               err_message(_("unable to read symlink: %s"), srcname);
+               goto quit;
+       }
+       linkbuf[i] = '\0';
+
+       if (realuid != 0 && realuid != st.st_uid) {
+               errno = EACCES;
+               err_open(srcname);
+               goto quit;
+       }
+
+       /* create target */
+       pname = strdup(srcname);
+       if (pname == NULL) {
+               err_nomem();
+               goto quit;
+       }
+       dirname(pname);
+
+       sprintf(target, "%s/%sXXXXXX", pname, cmd_prefix);
+       if (mktemp(target) == NULL) {
+               err_message(_("unable to create temp symlink name"));
+               goto quit;
+       }

do not create the file, it is done later with symlink, if the file
exists, symlink is going to fail.

+       cur_target = strdup(target);
+       if (cur_target == NULL) {
+               err_nomem();
+               goto quit;
+       }

cur_target is not needed.

+
+       if (symlink(linkbuf, target) != 0) {
+               err_message(_("unable to create symlink: %s"), target);
+               goto quit;
+       }

[...]

+       free(cur_target);
+
+       cur_target = NULL;


again, both are unnecesary.

+       numslinksdone++;
+       return rval;
+}



<Prev in Thread] Current Thread [Next in Thread>