On 12/30/2011 02:43 PM, 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.
Actually there is a syntax-check for this in coreutils. Hence there are no bad uses there. The snippet I used above was cut and pasted from a comment, which wasn't apparent as I used `git grep` to search for the preferred form of the idiom. I thought it a bit strange that there were no spaces around the operators but didn't investigate further :) cheers, Pádraig.
