Stephen Lau wrote: > Nathan Bush wrote: >> Stephen Lau wrote: >>> 2) We throw a "Header guard does not match filename" on a header >>> guard that looks like: __FOO_H__ (for foo.h) or other things that >>> don't strictly match _FOO_H_ >>> Is this the best error message? Intuitively, I would think it should >>> be "Invalid or missing header guard" myself - but this is the same >>> behaviour as the gate's hdrchk.pl, so I'm curious what other people >>> think. >> >> I think the two messages are used differently. "Invalid or missing >> header >> guard" is already used earlier in the code to indicate that the >> "#ident" line >> marking the start of the guard wasn't found where it was expected. >> "Header >> guard does not match filename" is used to indicate that the constant >> value >> found in the header does not match what was expected based on hdrchk's >> application of cstyle rules. > > Yeah, I realise the different contexts of use. It just seems to me that > __FOO_H__ not matching _FOO_H_ is a 'formatting' issue, rather than a > 'filename' issue. It's minor, and not a huge deal to me though.
I see your point. Perhaps you could reword the message slightly, such as changing "does not match filename" to "name does not follow suggested style" or something similar. >> Incidentally, I wonder if it is possible to tighten up hdrchk's logic >> here. >> For example, according to the cstyle rules, <zone.h> should use a >> guard of >> "_ZONE_H", while <sys/zone.h> should use a guard of "_SYS_ZONE_H". >> hdrchk >> looks only at the basename of the header file, and has a ".*" in the >> regex >> to cover any "SYS_", "NET_", etc. in the actual guard. So if there >> are two >> headers that share the same basename, it can't strictly enforce the >> rules. > > Hrm, this gets tricky in that you want to be able to hdrchk a file > without it necessarily being in its installed location, and the dirname > its in (in the source base) may not necessarily match up with its > installed location. (for the most part it looks like things in > uts/common/sys install to /usr/include/sys, so generally it seems to > hold true - but I wouldn't want to make that assumption for the whole tree) After thinking about this more, it seems that to do this level of checking, hdrchk would need to know the compiler's include file path, so it would know how much of the dirname prefix to ignore before trying to determine the exact expected header guard name. Since that's not available, it would only be able to guess, and that's probably not much better than just using the regex wildcard as it does now. --Nathan
