Jeff King <peff@xxxxxxxx> writes:
> But this one for example:
>
>> -#define flockfile(fh)
>> -#define funlockfile(fh)
>> +static inline void flockfile(FILE *fh)
>> +{
>> + ; /* nothing */
>> +}
>> +static inline void funlockfile(FILE *fh)
>> +{
>> + ; /* nothing */
>> +}
>
> is re-defining a name that's usually reserved for the system (at least
> on POSIX systems). For most systems that define it, we'd actually use
> the system version (and not compile this code at all).
> But there may be
> systems where we choose not to (either the system version is deficient,
> or we're testing the fallback code on a more-capable system, or our
> #ifndef check isn't sufficient on that system for some reason).
Hmph, perhaps. We'll cross that bridge when we need to port to such
a system, and until then, doing this will more easily catch the need
to cross that bridge, I would imagine.
> If the system defines it as a macro, we'd probably get a garbled
> compiler error as the macro is expanded here. Adding #undef flockfile,
> etc beforehand would help. I'm not sure if the current code might give
> us a macro redefinition warning on such a system.
>
> If the system defines it as a function, we'd probably get redefinition
> warnings.
>
> So...I dunno. Those are all theoretical complaints. But I also think
> what it's buying is not very big:
>
> - unlike precompose_argv(), modern POSIX-compliant systems (which we
> all tend to develop on) don't hit this fallback code. So your
> average developer is likely to see any problems here.
That's OK. I am not particularly interested in systems that has to
ifdef out flockfile() and funlockfile(). I did these two primarily
to reduce the number of instances of the bad pattern to implement a
no-op replacement as C-preprocessor macro that can be copied by less
experienced developers.
> - this is really the tip of the portability iceberg anyway. In the
> example that motivated this thread, you were catching failures to
> adjust to strvec. But in code like this:
>
> #ifdef FOO
> void some_func(int x, const char *y)
> {
> struct argv_array whatever = ARGV_ARRAY_INIT;
> ...
> }
> #else
> void some_func(int x, const char *y)
> {
> /* do nothing */
> }
> #endif
Yes, of course, but as I wrote in my response to Brian, the whole
point of using these replacement implementation macros is so that we
do not have to sprinkle the main code with such #ifdef/#endif, so
I think the code like that is what needs to be corrected ;-)
>> @@ -270,7 +272,9 @@ struct itimerval {
>> #endif
>>
>> #ifdef NO_SETITIMER
>> -#define setitimer(which,value,ovalue)
>> +static inline int setitimer(int which, const struct itimerval *value, struct itimerval *newvalue) {
>> + ; /* nothing */
>> +}
>> #endif
>
> Same reasoning applies to this one, plus the added bonus that we'd need
> that struct type defined. brian mentioned using "void *".
See my message to Brian.
