Nathan Bush wrote: > 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.
What about: "Header guard does not match suggested style (_FILENAME_H_)" Or is that too nitty? >>> 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. Agreed. I'll leave it as is for now. cheers, steve -- stephen lau // stevel at sun.com | 650.786.0845 | http://whacked.net opensolaris // solaris kernel development
