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

[Iordan Neshev]

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

                 Summary: Various changes
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: iordan_neshev
            Submitted on: Wed 05 May 2010 08:28:40 PM GMT
                Category: None
         Should Start On: Wed 05 May 2010 12:00:00 AM GMT
   Should be Finished on: Wed 05 May 2010 12:00:00 AM GMT
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
        Percent Complete: 0%
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
                  Effort: 0.00

    _______________________________________________________

Details:

I have several proposals:

1. In file pbuf.c, line 566 there is
    LWIP_ASSERT("pbuf_free: p->ref > 0", p->ref > 0);

Since p->ref is decremented on the next line, if underflow 
occurs, the assert would not fire. Casting to s32_t (or maybe s16_t to be
more lightweight?) should solve this:
    LWIP_ASSERT("pbuf_free: p->ref > 0", (s32_t)p->ref > 0);    /* cast to s32
to detect u16_t underflow */

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

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

there is:  ("PPP nPut: incomplete sio_write(fd:%d, len:%d, c: 0x%"X8_F") c =
%d\n", (size_t)pc->fd, b->len, c, c));

pc->fd should be casted to sio_fd_t, not size_t.

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

on the bottom there is:
 FSMDEBUG(LOG_INFO, ("%s: sending Configure-Request, id %d\n",

I  believe, we should rename "sending" to "sent", because it is quite
misleading when you have to read the PPP log, although pppd says "sending".
I'll send this to the pppd mailing list too.

5. ppp/fsm.c:  fsm_rtermreq(fsm *f, int id, u_char *p, int len)

There is
    case LS_OPENED:
      if (len > 0) {
        FSMDEBUG(LOG_INFO, ("%s terminated by peer (%p)\n", PROTO_NAME(f),
p));
      } else {
        FSMDEBUG(LOG_INFO, ("%s terminated by peer\n", PROTO_NAME(f)));
      }


I propose to change it like this:
      if ((len > 0) && (p != NULL)) {
         p[len] = '\0';         /* Terminate remote message */
         FSMDEBUG(LOG_INFO, ("%s terminated by peer (%s)\n", PROTO_NAME(f),
p));
      } else {
        FSMDEBUG(LOG_INFO, ("%s terminated by peer\n", PROTO_NAME(f)));
      }


I've seen it with my eyes that the remote message should be terminated
locally, otherwise random garbage appears after it. Also, p may be renamed to
msg, because we use p for pbuf, not for char*.

6. ppp/auth.c: auth_withpeer_fail(int unit, u16_t protocol)
In the latest pppd this function has these 2 rows added at the bottom:
    status = EXIT_PEER_AUTH_FAILED;
    lcp_close(unit, "Authentication failed");

We do not use currently status in PPP code, but I think
lcp_close(unit, "Authentication failed"); 
should be added at the end.

Sorry for not preparing diff files, but my lwip files are pretty different
from the CVS ones.




    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/