[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: cityhash - family of non-cryptographic hash functions for strings



On Wed, Jun 15, 2011 at 4:13 PM, Jakub Wilk <jwilk@debian.org> wrote:
> * Alessandro Ghedini <al3xbio@gmail.com>, 2011-06-10, 12:59:
>>>
>>> I saw this in city.h:
>>>
>>> | typedef uint8_t uint8;
>>> | typedef uint32_t uint32;
>>> | typedef uint64_t uint64;
>>> | typedef std::pair<uint64, uint64> uint128;
>>> |
>>> | inline uint64 Uint128Low64(const uint128& x) { return x.first; }
>>> | inline uint64 Uint128High64(const uint128& x) { return x.second; }
>>>
>>> It is not acceptable to define such a generic names in a public header.
>>> Please educate your upstream not to do that.
>>
>> I don't quite understand. Of course they may have just avoided those
>> typedefs in the first place but, no one forces you to include that header if
>> you don't find it appropriate.
>
> Great, so the bug doesn't affect user who don't use your package. How about
> caring also about those who do?

May I please make a simple suggestion to put "static inline" instead
of just "inline" in front of the questionable function declarations?
This will cause VERY annoying link-time problems if the header file is
ever #included into more than one object file.

By putting "static", the problem of generic names will be at least
limited to compile-time which is less serious and easier to work
around than link-time.

-David


Reply to: