[pisa-src] Re: r1640 - trunk/libpisa/checksum.h
- From: Thomas Jansen <mithi@xxxxxxxxx>
- To: pisa-src@xxxxxxxxxxxxx
- Date: Wed, 11 Nov 2009 14:59:49 +0100
On Wed, Nov 11, 2009 at 02:21:56PM +0100, Diego Biurrun wrote:
> > +/* TODO: needs to include a header for uint{16,32}_t to be selfcontained */
>
> The header that contains the typedefs for uint16_t?
> That header would be stdint.h.
Fixed in 1649, thanks.
> > typedef uint16_t pisa_csum16;
> > typedef uint32_t pisa_csum32;
>
> I think such typedefs are harmful obfuscation. Now if I read a function
> that uses pisa_csum32 I have to read some context to understand it
> completely. Why would we need such an extra layer of indirection?
I think it's useful to distinguish between a generic type (uint...) and the
type representing a certain use case for the generic type (pisa_csum...).
Especially when we have a function like
void pisa_csum_replace4(pisa_csum16 *addr, uint32_t before, uint32_t after)
it's easy to see that before and after are not checksums but actual data.
> > @@ -21,7 +23,7 @@
> > -static inline uint16_t pisa_csum_fold(pisa_csum32 c)
> > +static inline pisa_csum16 pisa_csum_fold(pisa_csum32 c)
>
> As I said, I think this was more readable before.
In that case you should complain about using pisa_csum32 as the type for the
argument. ;) I changed this to match the rest of the file, where the types
are used "properly". Unless we drop the typedefs altogether, we should be
consistent in their use.
--
Thomas Jansen, "Mithi" --- mithi@xxxxxxxxx
GPG 9D5C682B, feel free to sign or encrypt your mail
Other related posts:
