Re: mingw and same-inode

[Eric Blake]

-----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 <address@hidden>
>> 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             address@hidden
-----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-----