Shaun Jackman wrote:
>> 2, do you really need all these includes?
>> +#include <arpa/inet.h>
>> +#include <errno.h>
>> +#include <math.h>
>> +#include <stdint.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <time.h>
>> are all of these ones actually available on all platforms we support?
>
> At one point each of these includes was required. I'll double check to
> make sure they are all still relevant. They are all very standard
> includes. I would be quite surprised if a platform didn't include
> them. It certainly wouldn't be a POSIX platform.
>
Not all Wireshark platforms are POSIX, I don't think that Win32 provides
arpa/inet.h or stdint.h.
>> 3, not all compilers we support allow you to declare variables mid-block
>> + if (file_seek(wth->fh, 3, SEEK_CUR, err) == -1)
>> + return FALSE;
>> +
>> + uint8_t stream;
>>
>> 4, dont use these non-guintX types
>> uint8_t
>> use the g[u]int{8|16|32} from glib instead of these _t types.
>
> I prefer the standard uint8_t types from the Single UNIX Specification
> (SUSv3) and probably other major standards over some local flavour of
> guint8. It makes the code more readable and portable. I *really* don't
> see the point to prefixing standard symbols with the letter g because
> somebody decided to reinvent the wheel.
>
See comment above. The glib types help with a greater portability than
SUSv3.
--
Regards,
Graham Bloice