lwip-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lwip-devel] [bug #54062] Inconsequent null pointer checks


From: Jan Breuer
Subject: [lwip-devel] [bug #54062] Inconsequent null pointer checks
Date: Thu, 7 Jun 2018 09:40:38 -0400 (EDT)
User-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.62 Safari/537.36

Follow-up Comment #7, bug #54062 (project lwip):

Some notes to your patch

Adding function name as prefix to all error messages is nice but it makes code
unnecessary bigger. It is not a problem for debug messages but it can be a
problem for production builds.

LWIP_ASSERT and LWIP_ERROR can't be selectively disabled per functionality
like LWIP_DEBUGF can. So, you have them all or you don't have them at all. Be
careful by adding lot of new long texts.

There is still ability to implement custom LWIP_ERROR/LWIP_ASSERT which will
add macros like __FILE__, __LINE__, __func__ or any other text available by
your compiler.



There are some copy/paste bugs like
+  LWIP_ERROR("netif_set_addr, invalid netif", netif != NULL, return);
+  LWIP_ERROR("netif_set_addr, invalid *netif*", *netmask* != NULL, return);
+  LWIP_ERROR("netif_set_addr, invalid *netif*", *gw* != NULL, return);

There is inconsistency with starting the error message
+   LWIP_ASSERT("tcp_remove_listener: invalid pcb list", list != NULL);
vs
+  LWIP_ASSERT("netif_get_ip6_addr_match, invalid netif", netif != NULL);

Sometimes you use "," and sometimes ":". I think, that preferable is ":",
because it is already used in the source code.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?54062>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]