On Sep 17, 2008, at 12:09, Mark Phalan wrote: >> usr/src/lib/gss_mechs/mech_krb5/crypto/des/afsstring2key.c: >> 389: this line deviates from MIT's probably because of a lint fix, >> should revert (w/o NULL)? > > I think lint will complain but I'll play with it.
I really should track down info on how to look at the changes on your site :-) ... but if it's a minor enough change we can probably take it upstream. (There may be cases of our "coding standards", vague and sometimes unwritten as they are, not quite agreeing with what lint wants; some of our approach is "make gcc not complain quite so much". I played with lint a little bit too but ran into some bugs, which I put into the reporting pipeline here on campus. But we are planning to move towards more tool-based checking of the code, and enforcement of consistent standards; just not 100% clear yet what those standards will be.) >> 441: this was changed because of "PSARC/2004/619 Namespace for >> Solaris >> on amd64", >> are the consequences know for reverting back? > > The "register" keyword is simply a way to advise the compiler that the > variable will be heavily used (so it might be good to keep it in a > register). It would seem to be suitable in this situation and its what > MIT uses. > I don't have any performance comparisons of the compiled code. I don't > mind reverting if anyone has a strong opinion. So the Sun compiler uses "register"? I believe gcc generally ignores it, and so we've sometimes been dropping it from the MIT sources. In any case, the performance of string-to-key functions is probably not very important (client uses them at login time, kadmind uses them at password-change time), and the performance of the AFS one probably least so. If your customers are complaining about performance in specific areas, I'd like to know what they are. We have a few arbitrary performance test programs we don't use much, and no particular areas where we're watching performance as we make changes. (We do know of some specific problem areas in the MIT code, like replay cache performance and DNS delays.) >> usr/src/lib/gss_mechs/mech_krb5/include/socket-utils.h: >> 77,82,87,91,96: why aren't these designated as inline? > > gcc complains about "inline" when compiled on Solaris (perhaps due to > the opts we pass it?). That code however isn't used to genereate the > final library (as its gcc only) so it doesn't really make much > difference. > I've reverted anyway to the previous syntax. Sadly, the various "inline" options differ between gcc and c99, and to make it more confusing, gcc is changing its behavior to comply with the standard, so saying "gcc" doesn't imply just one behavior. We use "static inline" for lots of stuff currently, because gcc will discard the static function if it doesn't need to be called by address, we won't get symbol name conflicts, and the name won't be exported from the library. But the Sun compiler keeps the static copy around, leading to bloat. I'm leery of non-static versions that export the symbol names, as we haven't named all the functions as part of "our" part of the symbol name space. If we know an export list used to build the library will keep the function restricted to library- internal use, and conflicts aren't a problem, "inline" or "extern inline" might be better. I'd really like to come up with a solution that makes the code more adaptable. E.g., use "INLINE" and define that as necessary on each platform based on the compiler, whether we're building a shared library, whether we can use export lists, etc. But it hasn't made it to the top of my work queue yet, as we've got gcc on all our UNIX systems, and not too many complaints about code size.... Ken
