James,
I guess you take over.
There are many lines that does not change anything.
Also, maybe it would be better to have one string variable and one
type, (gateway, device) (gateway, address), so that it would be easier
to manage if new type comes up.

Alon.

On 2/3/09, David Balazic <david.bala...@hermes-softlab.com> wrote:
> James Yonan wrote:
>
>  > David,
>  >
>  > A couple issues with the patch:
>  >
>  > * sscanf usage doesn't check for buffer overflow.
>
>  See below.
>
>  > * You use gw_if_name in some places and gw_ifname in other
>  > places.  To
>  > eliminate confusion it would be best to use a consistent form.
>
>  Done.
>
>  > Also, to reiterate, try submitting the patch as an attachment to deal
>  > with email client munging issues.
>
>  OK.
>
>  About the sscanf:
>  (copied from my earlier message to Alon)
>
> > 3. Please don't use scanf this way as it may overflow the buffer.
>  > Use %##s and check that overflow is avoided.
>
>  I use "%32s\t" now.
>  How do I check for overflow? sscanf always returns 1.
>  Besides, the linux kernel headers declare 32 as maximum iface name
>  length.
>  Even in case of overflow, what shoudl I do ? Return false ?
>  I think leaving it as is is good for 99,999% of cases and in the rest
>  (in case of overflow), either a gateway IP address is present, so no
>  problem, or the route add command will fail (or use the wrong device).
>  Not perfect, but what is in these days ? ;-)
>
>
> Oh, my patch removes spaces at the end of some lines. My editor did
>  this automatically. Is that a problem ? I thought to remove it from
>  the patch, but then the next guy to edit it will have the same problem.
>
>  The patch is attached.
>
>  Regards,
>
> David
>
>

Reply via email to