Re: [PATCH] canonicalize: add support for not resolving symlinks

[Jim Meyering]

Jim Meyering wrote:

> Pádraig Brady wrote:
>> On 12/30/2011 01:49 PM, Eric Blake wrote:
>>> On 12/30/2011 04:04 AM, Jim Meyering wrote:
>>>>> -          if (lstat (rname, &st) != 0)
>>>>> +          if ((logical?stat:lstat) (rname, &st) != 0)
>>>>
>>>> Please add spaces around operators:
>>>>
>>>>              if ((logical ? stat : lstat) (rname, &st) != 0)
>>>
>>> Better yet, don't write this as a function pointer conditional, as the
>>> gnulib replacement for stat on some platforms has to be a function-like
>>> macro (no thanks to 'struct stat').  Using a function pointer
>>> conditional risks missing the gnulib macro for stat, and calling the
>>> underlying system stat() instead of the intended rpl_stat(), for broken
>>> behavior.  You have to use:
>>>
>>> if (logical ? stat (rname, &st) : lstat (rname, &st)) != 0)
>>>
>>
>> Ouch. Good catch.
>> I'll need to fix in a follow up patch.
>> I'll need to adjust such uses in coreutils too.
>
> Oh!  I'd just finished checking gnulib for other instances,
> then, after seeing your message, did this in coreutils:
>
>   $ git grep -E '\<l?stat *: *l?stat\>'
>   src/ln.c:          (logical?stat:lstat) (source, &source_stats)
>   src/stat.c:     (follow_links?stat:lstat) (filename, &statbug)
>   src/touch.c:      /* Don't use (no_dereference?lstat:stat) (args), since 
> stat
>
> Here's a proposed addition to gnulib's maint.mk:
>
>>From 24b71909a0b4022aeeea7fa74f250afa243e8d7a Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Fri, 30 Dec 2011 16:06:33 +0100
> Subject: [PATCH] maint.mk: add a syntax check for subtle stat/lstat usage
>  problem
>
> * top/maint.mk (sc_stat_lstat_without_open_paren): New rule.
> Motivated by this exchange:
> http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/29491/focus=29496
> ---
>  ChangeLog    |    5 +++++
>  top/maint.mk |   10 ++++++++++

Retracted.
As Pádraig pointed out, coreutils already has a test for this in its cfg.mk.
As for whether it should migrate to gnulib's maint.mk is another story.
Considering there are 3 bad-code comments in coreutils that would
trigger it, I'm considering obfuscating those comments.
It'd be nice to remove them, but that would expose us to the risk of
"factorization" with the goal of avoiding the duplicate argument lists.