[haiku-gsoc] Re: Fwd: Re: [HCD08] ICMP error handling & propagation project
- From: "Yin Qiu" <allenchue@xxxxxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Mon, 15 Sep 2008 16:55:08 +0800
Hi Axel,
Here's the updated patch.
I have corrected most of the issued you mentioned except the following:
>> --- src/add-ons/kernel/network/protocols/ipv4/ipv4.cpp (revision
>> 27207)
>> +++ src/add-ons/kernel/network/protocols/ipv4/ipv4.cpp (working
>> copy)
> [...]
>> net_timer fTimer;
>> + net_buffer *fLastBuffer;
>> };
>
> Indentation is wrong: we use a tab size of 4.
Sorry but I don't know what you mean here by a tab size of 4.
>> +++ src/add-ons/kernel/network/protocols/icmp/icmp.cpp (working
>> copy)
> [...]
>> + uint32 gateway;
>> struct {
>> - in_addr_t gateway;
>
> in_addr_t should be the type of this member here, as it contains an
> address, not some arbitrary number.
The gateway field here is just for convenient manipulation, not only
stands for the gateway information in a Redirect message. It
represents the 32-bit content in an ICMP header, no matter the actual
content is the gateway, unused, or (identifier+sequence).
>> +++ headers/private/net/ipv4.h (revision 0)
> [...]
>> +} _PACKED;
>> +
>> +
>> +#define IP_BUFFER_HIGH_WATERMARK 0.9
>> +
>> +
>> +#define IP_VERSION 4
>> +
>> +#endif
>
> The last #endif misses the /* IPV4_H */. Shared system headers like
> this one should get an extra '_' in front of their header guard.
> Also, please don't use white space and indentation so inconsistently.
> And BTW, it's 2008 already (all your new headers have 2006-2007
> copyright, but should only have 2008).
> icmp.h looks like it wasn't written with a tab size of 4 either, and
> shares the same problems as ipv4.h.
Again I don't understand which part of the code you are referring to
when saying "a tab size of 4".
The technical review may be more miserable :) I'll try my best to
catch up with. Anyway thanks for your effort.
--
Yin Qiu
Nanjing University, China
-------------------------------------------
Other related posts:
