Wireshark · Wireshark-dev: Re: [Wireshark-dev] [PATCH 1/2] wiretap: New MPEG file format

[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

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