threads / patch / 502

patchfix compare symlink against readlink not data

Subject: [PATCH] fix compare symlink against readlink not data

## tl;dr

9 messages between May 6, 2005 and May 6, 2005. Diffs are folded; open one to read it.

replies: 8people: 4as markdown or json

Kay Sievers· May 6, 2005, 13:45 UTC · lore

Fix update-cache to compare the blob of a symlink against the link-target and not the file it points to. Also ignore all permissions applied to links. Thanks to Greg for recognizing this while he added our list of symlinks back to the udev repository.

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
Show changes to 3 files +48 −9

diff-files.c, read-cache.c, update-cache.c

--- a/diff-files.c
+++ b/diff-files.c
@@ -111,7 +111,7 @@ int main(int argc, char **argv)
 			continue;
 		}
  
-		if (stat(ce->name, &st) < 0) {
+		if (lstat(ce->name, &st) < 0) {
 			if (errno != ENOENT) {
 				perror(ce->name);
 				continue;
--- a/read-cache.c
+++ b/read-cache.c
@@ -16,6 +16,9 @@ int cache_match_stat(struct cache_entry 
 	switch (ntohl(ce->ce_mode) & S_IFMT) {
 	case S_IFREG:
 		changed |= !S_ISREG(st->st_mode) ? TYPE_CHANGED : 0;
+		/* We consider only the owner x bit to be relevant for "mode changes" */
+		if (0100 & (ntohl(ce->ce_mode) ^ st->st_mode))
+			changed |= MODE_CHANGED;
 		break;
 	case S_IFLNK:
 		changed |= !S_ISLNK(st->st_mode) ? TYPE_CHANGED : 0;
@@ -43,9 +46,6 @@ int cache_match_stat(struct cache_entry 
 	if (ce->ce_uid != htonl(st->st_uid) ||
 	    ce->ce_gid != htonl(st->st_gid))
 		changed |= OWNER_CHANGED;
-	/* We consider only the owner x bit to be relevant for "mode changes" */
-	if (0100 & (ntohl(ce->ce_mode) ^ st->st_mode))
-		changed |= MODE_CHANGED;
 	if (ce->ce_dev != htonl(st->st_dev) ||
 	    ce->ce_ino != htonl(st->st_ino))
 		changed |= INODE_CHANGED;
--- a/update-cache.c
+++ b/update-cache.c
@@ -64,7 +64,7 @@ static int add_file_to_cache_1(char *pat
 	struct stat st;
 	int fd;
 	unsigned int len;
-	char target[1024];
+	char *target;
 
 	if (lstat(path, &st) < 0) {
 		if (errno == ENOENT || errno == ENOTDIR) {
@@ -90,11 +90,14 @@ static int add_file_to_cache_1(char *pat
 			return -1;
 		break;
 	case S_IFLNK:
-		len = readlink(path, target, sizeof(target));
-		if (len == -1 || len+1 > sizeof(target))
+		target = xmalloc(st.st_size+1);
+		if (readlink(path, target, st.st_size+1) != st.st_size) {
+			free(target);
 			return -1;
-		if (write_sha1_file(target, len, "blob", ce->sha1))
+		}
+		if (write_sha1_file(target, st.st_size, "blob", ce->sha1))
 			return -1;
+		free(target);
 		break;
 	default:
 		return -1;
@@ -163,6 +166,32 @@ static int compare_data(struct cache_ent
 	return match;
 }
 
+static int compare_link(struct cache_entry *ce, unsigned long expected_size)
+{
+	int match = -1;
+	char *target;
+	void *buffer;
+	unsigned long size;
+	char type[10];
+	int len;
+	target = xmalloc(expected_size);
+	len = readlink(ce->name, target, expected_size);
+	if (len != expected_size ) {
+		free(target);
+		return -1;
+	}
+	buffer = read_sha1_file(ce->sha1, type, &size);
+	if (!buffer) {
+		free(target);
+		return -1;
+	}
+	if (size == expected_size)
+		match = memcmp(buffer, target, size);
+	free(buffer);
+	free(target);
+	return match;
+}
+
 /*
  * "refresh" does not calculate a new sha1 file or bring the
  * cache up-to-date for mode/content changes. But what it
@@ -194,8 +223,18 @@ static struct cache_entry *refresh_entry
 	if (changed & (MODE_CHANGED | TYPE_CHANGED))
 		return ERR_PTR(-EINVAL);
 
-	if (compare_data(ce, st.st_size))
+	switch (st.st_mode & S_IFMT) {
+	case S_IFREG:
+		if (compare_data(ce, st.st_size))
+			return ERR_PTR(-EINVAL);
+		break;
+	case S_IFLNK:
+		if (compare_link(ce, st.st_size))
+			return ERR_PTR(-EINVAL);
+		break;
+	default:
 		return ERR_PTR(-EINVAL);
+	}
 
 	cache_changed = 1;
 	size = ce_size(ce);
Greg KH· May 6, 2005, 16:03 UTC · re: Kay Sievers · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, May 06, 2005 at 03:45:01PM +0200, Kay Sievers wrote:
Show 5 quoted lines
> Fix update-cache to compare the blob of a symlink against the link-target
> and not the file it points to. Also ignore all permissions applied to
> links.
> Thanks to Greg for recognizing this while he added our list of symlinks
> back to the udev repository.
Hm, even with this patch applied (it's in Linus's tree right now), I
still get the following with a clean checked out udev tree:
 $ cg-diff
 Index: test/sys/block/cciss!c0d0/device
 ===================================================================
 Index: test/sys/block/rd!c0d0/device
 ===================================================================
 Index: test/sys/block/sda/device
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:00:09.0
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:00:1e.0
 ===================================================================
 Index: test/sys/bus/pci/devices/0000:02:05.0
 ===================================================================
 Index: test/sys/bus/pci/drivers/aic7xxx/0000:02:05.0
 ===================================================================
 Index: test/sys/bus/scsi/devices/0:0:0:0
 ===================================================================
 Index: test/sys/bus/scsi/drivers/sd/0:0:0:0
 ===================================================================
 Index: test/sys/bus/usb-serial/devices/ttyUSB0
 ===================================================================
 Index: test/sys/bus/usb-serial/drivers/PL-2303/ttyUSB0
 ===================================================================
 Index: test/sys/bus/usb/devices/3-0:1.0
 ===================================================================
 Index: test/sys/bus/usb/devices/3-1
 ===================================================================
 Index: test/sys/bus/usb/devices/3-1:1.0
 ===================================================================
 Index: test/sys/bus/usb/devices/usb3
 ===================================================================
 Index: test/sys/bus/usb/drivers/hub/3-0:1.0
 ===================================================================
 Index: test/sys/bus/usb/drivers/pl2303/3-1:1.0
 ===================================================================
 Index: test/sys/bus/usb/drivers/usb/3-1
 ===================================================================
 Index: test/sys/bus/usb/drivers/usb/usb3
 ===================================================================
 Index: test/sys/class/tty/ttyUSB0/device
 ===================================================================
 Index: test/sys/devices/pci0000:00/0000:00:09.0/usb3/3-1/ttyUSB0/driver
 ===================================================================
Any idea?
thanks,
greg k-h
Kay Sievers· May 6, 2005, 16:23 UTC · re: Greg KH · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, 2005-05-06 at 09:03 -0700, Greg KH wrote:
Show 12 quoted lines
> On Fri, May 06, 2005 at 03:45:01PM +0200, Kay Sievers wrote:
> > Fix update-cache to compare the blob of a symlink against the link-target
> > and not the file it points to. Also ignore all permissions applied to
> > links.
> > Thanks to Greg for recognizing this while he added our list of symlinks
> > back to the udev repository.
> 
> Hm, even with this patch applied (it's in Linus's tree right now), I
> still get the following with a clean checked out udev tree:
>  $ cg-diff
>  Index: test/sys/block/cciss!c0d0/device
>  ===================================================================

I can't reproduce this. Are you sure, that the git-core binaries are called and not the cogito ones?

  git-update-cache --refresh
  git-diff-cache -r HEAD
from the core-git should print nothing.

Thanks, Kay

Greg KH· May 6, 2005, 16:36 UTC · re: Kay Sievers · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, May 06, 2005 at 06:23:34PM +0200, Kay Sievers wrote:
Show 20 quoted lines
> On Fri, 2005-05-06 at 09:03 -0700, Greg KH wrote:
> > On Fri, May 06, 2005 at 03:45:01PM +0200, Kay Sievers wrote:
> > > Fix update-cache to compare the blob of a symlink against the link-target
> > > and not the file it points to. Also ignore all permissions applied to
> > > links.
> > > Thanks to Greg for recognizing this while he added our list of symlinks
> > > back to the udev repository.
> > 
> > Hm, even with this patch applied (it's in Linus's tree right now), I
> > still get the following with a clean checked out udev tree:
> >  $ cg-diff
> >  Index: test/sys/block/cciss!c0d0/device
> >  ===================================================================
> 
> I can't reproduce this. Are you sure, that the git-core binaries are
> called and not the cogito ones?
> 
>   git-update-cache --refresh
>   git-diff-cache -r HEAD
> from the core-git should print nothing.

Odd. If I reclone the whole tree from the udev kernel.org tree, then it works just fine. If I create a new copy of my local tree, I still have the same problem. Diffing the trees shows no difference in the objects at all...

Can you add a symlink to a local tree and see if you can duplicate this?
thanks,
greg k-h
Kay Sievers· May 6, 2005, 16:59 UTC · re: Greg KH · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, 2005-05-06 at 09:36 -0700, Greg KH wrote:
Show 28 quoted lines
> On Fri, May 06, 2005 at 06:23:34PM +0200, Kay Sievers wrote:
> > On Fri, 2005-05-06 at 09:03 -0700, Greg KH wrote:
> > > On Fri, May 06, 2005 at 03:45:01PM +0200, Kay Sievers wrote:
> > > > Fix update-cache to compare the blob of a symlink against the link-target
> > > > and not the file it points to. Also ignore all permissions applied to
> > > > links.
> > > > Thanks to Greg for recognizing this while he added our list of symlinks
> > > > back to the udev repository.
> > > 
> > > Hm, even with this patch applied (it's in Linus's tree right now), I
> > > still get the following with a clean checked out udev tree:
> > >  $ cg-diff
> > >  Index: test/sys/block/cciss!c0d0/device
> > >  ===================================================================
> > 
> > I can't reproduce this. Are you sure, that the git-core binaries are
> > called and not the cogito ones?
> > 
> >   git-update-cache --refresh
> >   git-diff-cache -r HEAD
> > from the core-git should print nothing.
> 
> Odd.  If I reclone the whole tree from the udev kernel.org tree, then it
> works just fine.  If I create a new copy of my local tree, I still have
> the same problem.  Diffing the trees shows no difference in the objects
> at all...
> 
> Can you add a symlink to a local tree and see if you can duplicate this?
Works as it should be.
What happens when you throw away your old .git/index with?
  git-read-tree HEAD
Kay
Linus Torvalds· May 6, 2005, 17:11 UTC · re: Greg KH · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, 6 May 2005, Greg KH wrote:
Show 5 quoted lines
> 
> Odd.  If I reclone the whole tree from the udev kernel.org tree, then it
> works just fine.  If I create a new copy of my local tree, I still have
> the same problem.  Diffing the trees shows no difference in the objects
> at all...
You've not updated your cache.
Guys, remember this command:
	git-diff-files

Just like that, with no arguments. It shows you what is different in your cache. If you get a lot of output, it means that your index file isn't up-to-date.

The other magic command is
	git-update-cache --refresh
and you need to do that after you merge.

If you use cogito, and cogito doesn't refresh after pulls etc, that would be a cogito bug. But if you do things like "cp -a" of a git tree, and you forget to refresh the cache in the new tree, than that is _your_ bug..

		Linus
Greg KH· May 6, 2005, 17:19 UTC · re: Linus Torvalds · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, May 06, 2005 at 10:11:49AM -0700, Linus Torvalds wrote:
Show 22 quoted lines
> 
> 
> On Fri, 6 May 2005, Greg KH wrote:
> > 
> > Odd.  If I reclone the whole tree from the udev kernel.org tree, then it
> > works just fine.  If I create a new copy of my local tree, I still have
> > the same problem.  Diffing the trees shows no difference in the objects
> > at all...
> 
> You've not updated your cache.
> 
> Guys, remember this command:
> 
> 	git-diff-files
> 
> Just like that, with no arguments. It shows you what is different in your 
> cache. If you get a lot of output, it means that your index file isn't 
> up-to-date.
> 
> The other magic command is
> 
> 	git-update-cache --refresh

Damm, I still was using update-cache and checkout-cache from an old git version. That was my problem.

Sorry for the noise, it works just fine.
thanks,
greg k-h
Junio C Hamano· May 6, 2005, 16:26 UTC · re: Greg KH · lore

Re: [PATCH] fix compare symlink against readlink not data

>>>>> "GKH" == Greg KH <greg@kroah.com> writes:
GKH> On Fri, May 06, 2005 at 03:45:01PM +0200, Kay Sievers wrote:
>> Thanks to Greg for recognizing this while he added our list of symlinks
>> back to the udev repository.

GKH> Hm, even with this patch applied (it's in Linus's tree right now), I GKH> still get the following with a clean checked out udev tree: GKH> $ cg-diff GKH> Index: test/sys/block/cciss!c0d0/device GKH> =================================================================== GKH> Index: test/sys/block/rd!c0d0/device GKH> ===================================================================

GKH> Any idea?

I do not use Cogito but probably it is this piece of code in cg-Xdiffdo. It is assuming that a valid SHA1 means it can diff against the filesystem object that resides there. It used to be the case before symlinks but not anymore.

    mkbanner () {
            loc=$1; treeid=$2; fname=$3; mode=$4; sha1=$5;
            if [ "$sha1" != "0000000000000000000000000000000000000000" ]; then
                    git-cat-file blob $sha1 >$loc
            else
                    ln -s "$(pwd)/$fname" "$loc"
                    sha1="!"
            fi

Maybe changing the if to (I'm writing this in e-mail editor so completely untested) something like:

    if expr "$sha1" : '0*$' >/dev/null ||
       expr "$mode" : '.*120000$' >/dev/null
    then
        git-cat-file blob "$sha1" >$loc
would help?

Also could you try the low-level git command, git-diff-cache -p, against the tree you are comparing? The built-in diff stuff might get this wrong too.

Greg KH· May 6, 2005, 16:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] fix compare symlink against readlink not data

On Fri, May 06, 2005 at 09:26:15AM -0700, Junio C Hamano wrote:
> 
> Also could you try the low-level git command, git-diff-cache -p,
> against the tree you are comparing?  The built-in diff stuff
> might get this wrong too.
No, 'git-diff-cache -r HEAD' shows me:

$ git-diff-cache -r HEAD *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/cciss!c0d0/device *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/rd!c0d0/device *120000->100644 blob 2d78258b1a0fe49afabc8c16a352117df5dc338a->2d78258b1a0fe49afabc8c16a352117df5dc338a test/sys/block/sda/device *120000->100644 blob 1c776568bdc9dc750addd0885dded6b008a44460->1c776568bdc9dc750addd0885dded6b008a44460 test/sys/bus/pci/devices/0000:00:09.0 *120000->100644 blob e000c77614a23ad57fed284bd007ed7c1cb7872e->e000c77614a23ad57fed284bd007ed7c1cb7872e test/sys/bus/pci/devices/0000:00:1e.0 *120000->100644 blob 630d35bf617944a4ba6afc90ca5176cb342a2662->630d35bf617944a4ba6afc90ca5176cb342a2662 test/sys/bus/pci/devices/0000:02:05.0 *120000->100644 blob bd644e0e9d0c2f289bc4a3e3a034d528d5d671cc->bd644e0e9d0c2f289bc4a3e3a034d528d5d671cc test/sys/bus/pci/drivers/aic7xxx/0000:02:05.0 *120000->100644 blob ebb65b3bac36ef935a55a7f1010e4d3a242188eb->ebb65b3bac36ef935a55a7f1010e4d3a242188eb test/sys/bus/scsi/devices/0:0:0:0 *120000->100644 blob 239003f712dd9112171e635a44160da1898f5996->239003f712dd9112171e635a44160da1898f5996 test/sys/bus/scsi/drivers/sd/0:0:0:0 *120000->100644 blob b7733a68e08e564300212a22c9f81888c12bb55a->b7733a68e08e564300212a22c9f81888c12bb55a test/sys/bus/usb-serial/devices/ttyUSB0 *120000->100644 blob 177f109e4899cf4008b9413933392d4f07832fdc->177f109e4899cf4008b9413933392d4f07832fdc test/sys/bus/usb-serial/drivers/PL-2303/ttyUSB0 *120000->100644 blob 9137978832942ecce572d376f14244c1588748a2->9137978832942ecce572d376f14244c1588748a2 test/sys/bus/usb/devices/3-0:1.0 *120000->100644 blob e47b4d58c4e5406bdba3ea1384c0c3efe007b8f6->e47b4d58c4e5406bdba3ea1384c0c3efe007b8f6 test/sys/bus/usb/devices/3-1 *120000->100644 blob f519185eb36af29f79ca89d4b3d51011756b6837->f519185eb36af29f79ca89d4b3d51011756b6837 test/sys/bus/usb/devices/3-1:1.0 *120000->100644 blob fb1919e7c9794ce31a257b50621f71f6f4f8bdef->fb1919e7c9794ce31a257b50621f71f6f4f8bdef test/sys/bus/usb/devices/usb3 *120000->100644 blob 2bc160c20cd950c52e34d4bab30e1e25d6f4df34->2bc160c20cd950c52e34d4bab30e1e25d6f4df34 test/sys/bus/usb/drivers/hub/3-0:1.0 *120000->100644 blob 49d32d5abd7e26766f4c905f1d4edf1e28f8b322->49d32d5abd7e26766f4c905f1d4edf1e28f8b322 test/sys/bus/usb/drivers/pl2303/3-1:1.0 *120000->100644 blob 03c76193e99a93c7ff45c9ac986d2bc8e0706b0b->03c76193e99a93c7ff45c9ac986d2bc8e0706b0b test/sys/bus/usb/drivers/usb/3-1 *120000->100644 blob 61dc52a61345178c8c171ecfe96df9646af2f16c->61dc52a61345178c8c171ecfe96df9646af2f16c test/sys/bus/usb/drivers/usb/usb3 *120000->100644 blob b7733a68e08e564300212a22c9f81888c12bb55a->b7733a68e08e564300212a22c9f81888c12bb55a test/sys/class/tty/ttyUSB0/device *120000->100644 blob 9ff2c81f529a95bd93ddaf66de6a72c74166c268->9ff2c81f529a95bd93ddaf66de6a72c74166c268 test/sys/devices/pci0000:00/0000:00:09.0/usb3/3-1/ttyUSB0/driver

thanks,
greg k-h

← back to recent threads