[lwip-devel] [task #10369] Various changes

[Simon Goldschmidt]

Follow-up Comment #1, task #10369 (project lwip):

> 1. In file pbuf.c, line 566 there is ...

I don't think that casting to s32_t changes anything since it would result in
an s32_t between 0 and 0x0000ffff, which would never be negative. Casting to
s16_t might work, but would limit the current ref-count ot 32K, which
shouldn't be a problem, but still, if it was meant like that, I'd rather
change the structure definition to s16_t.

> 2. ppp_oe.c
> sys_untimeout should be UNTIMEOUT (lines 286, 618, 631 and 1211)

Hm, ppp_oe.c is a file written only for lwIP and my opinion is that it
therefore doesn't have to use the compatibility-defines (UN)TIMEOUT, which
were introduced for the pppd code (where TIMEOUT first calls sys_untimeout and
then sys_timeout). (After all, *if* UNTIMEOUT was used, than we would have to
use TIMEOUT, too...)

> 3. ppp.c : nPut(PPPControl pc, struct pbuf nb)

Hmm, that one is tricky. The current cast isn't correct since size_t is
unsigned (and may be larger than expected for %d) while %d expects
(signed-)int. However, we don't have any way of knowing which printf-formatter
is correct for sio_fd_t, so for fixing this correctly, we would have to
introduce a SIO_FD_X define.

> 4. ppp/fsm.c: function fsm_sconfreq(fsm *f, int retransmit)

I'd agree with you there, but that's not the only place where the present
progressive is used instead of the simple past, and I wouldn't just change it
in one place...

> 5. ...

Wouldn't we write beyond the buffer (and thus overwrite another memory area)
with p[len] = 0 if len is the size of the buffer that p points to???

Aside from that, I wouldn't want to change that unless it's in changed in
pppd: I'm not planning lwIP's ppp to become a real fork of the original pppd
with lots of changes (after all, if I had the time, I'd completely upgrade to
2.4.5 and use ifdef/macros to disable the stuff we don't need...).

> 6. ...

I've added the call to lcp_close. In lwIP, the state is changed by calling
pppIOCtl, so this is already covered.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/task/?10369>

_______________________________________________
  Nachricht geschickt von/durch Savannah
  http://savannah.nongnu.org/