This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix location where math-vector-fortran.h is installed.


* Zack Weinberg:

> Revised patch looks good to me except:
>
>> -! Platform-specific declarations of SIMD math functions for Fortran. -*- f90 -*-
>> +! Platform-specific declarations of SIMD math functions for Fortran.
>
> Let's keep the -*- f90 -*- annotations, they will also be helpful for
> future people editing these files.  For instance, on my computer, both
> emacs and vim use the Fortran syntax highlighting rules for these
> files when the annotation is present, and the C rules when it's
> absent.
>
>> +            # Skip Fortran header files
>> +            if 'finclude' in header:
>> +                continue
>
> I think this conditional should be
>
>     if header.startswith('finclude/') or '/finclude/' in header
>
> so that it only applies to headers in the directory 'finclude', not
> any hypothetical future headers that happen to have the string
> 'finclude' in their base names.  (I don't know why that would happen,
> but if it ever did, I can see someone being really confused why it
> wasn't getting tested, so some extra defensiveness now seems like a
> good idea.)

I agree.  Maybe also add a period to the end of the comment. 8-)

Thanks,
Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]