-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Jim Meyering on 9/24/2009 12:29 AM:
>> I'm also preparing a followup patch for coreutils usage of SAME_INODE.
>> Thoughts before I apply this?
>
> You have pushed these changes already.
> Imagine my surprise upon seeing unreviewed and unapproved API changes...
>
> Accidentally, no doubt, since I would expect you to wait at least
> for an "ok" from me before making API-changing changes to such
> fundamental modules.
Indeed; they were queued prior to my linkat changes. I've now pushed a
reversion commit; linkat now fails on mingw, but will be fixed again once
we decide the proper fix for same-inode.
>
> I haven't reviewed everything yet, but this error
> jumped out at me:
>
> bool
> same_name (const char *source, const char *dest)
> {
> ...
> bool same = false;
> ...
> same = SAME_INODE (source_dir_stats, dest_dir_stats);
> if (same < 0)
> same = (identical_basenames
> && strcmp (source_basename, dest_basename) == 0);
> ...
> return same;
> }
>
> Your new test, "same < 0" will never be true when
> compiled on a system with proper "bool" support.
>
> Apparently, since your testing did not expose this flaw,
> no tested use of same_name requires the new semantics of SAME_INODE.
>
> I have profound doubts about whether this is a desirable change.
> Since it is solely in support of non-POSIX platforms,
> I find it debatable, to say the least. You've converted the
> clearly-boolean SAME_INODE to be tri-state, with no way (other
> than inspection) for callers to detect the need to update
> their code.
>
> If the tri-state test is required in only a few places,
> how about using a new macro, say SAME_INODE_TRISTATE,
> in just those few places?
> With a name that makes it obvious the result is not boolean,
> there is far less risk of misuse.
>
>> >From 5538c910e1acd307a3d3e69f6d80044b1f3d935a Mon Sep 17 00:00:00 2001
>> From: Eric Blake <e...@byu.net>
>> Date: Wed, 23 Sep 2009 14:51:29 -0600
>> Subject: [PATCH 2/2] same-inode: make SAME_INODE tri-state, to port to mingw
>>
>> Mingw has the annoying habit (already documented in
>> doc/posix-functions/*stat) that st_ino is always 0.
>> This means that naive uses of SAME_INODE(a,b) would
>> succeed, even on distinct files.
> ...
>> diff --git a/NEWS b/NEWS
>> index 62c631f..87fc884 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -6,6 +6,9 @@ User visible incompatible changes
>>
>> Date Modules Changes
>>
>> +2009-09-23 same-inode The macro SAME_INODE is now tri-state, adding -1
>> + for unknown.
>> +
> ...
>> diff --git a/lib/same.c b/lib/same.c
>> index af3a95e..5251fb8 100644
>> --- a/lib/same.c
>> +++ b/lib/same.c
>> @@ -1,7 +1,7 @@
>> /* Determine whether two file names refer to the same file.
>>
>> - Copyright (C) 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006 Free
>> - Software Foundation, Inc.
>> + Copyright (C) 1997, 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006,
>> + 2009 Free Software Foundation, Inc.
>>
>> This program is free software: you can redistribute it and/or modify
>> it under the terms of the GNU General Public License as published by
>> @@ -97,6 +97,9 @@ same_name (const char *source, const char *dest)
>> }
>>
>> same = SAME_INODE (source_dir_stats, dest_dir_stats);
>> + if (same < 0)
>> + same = (identical_basenames
>> + && strcmp (source_basename, dest_basename) == 0);
>>
>> #if ! _POSIX_NO_TRUNC && HAVE_PATHCONF && defined _PC_NAME_MAX
>> if (same && ! identical_basenames)
>
- --
Don't work too hard, make some time for fun as well!
Eric Blake e...@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkq7XXgACgkQ84KuGfSFAYAf5gCZAQcJDAUzyqjSFVyMfEFha0Bo
ESUAnRdaH7Ezb5GT11GYoW+sEikL4z6Z
=+5eF
-----END PGP SIGNATURE-----
