buglwIP - A Lightweight TCP/IP stack - Bugs: bug #63324, memcpy(), memmove() and other libc...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #63324: memcpy(), memmove() and other libc dependencies

Submitter:  Faidon Liambotis <paravoid>
Submitted:  Sun 06 Nov 2022 07:13:37 PM UTC
   
 
Category:  Platform ports Severity:  3 - Normal
Item Group:  Feature Request Status:  Works For Me
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Wed 11 Oct 2023 07:08:32 PM UTC, comment #1: 

I can understand your problem. However, this doesn't fit into our "bugs" category in the bugtracker: you want new features, not only memcpy and memmove but also new abstractions for other functions.

Please feel free to provide a patch doing this (and possibly fixing usages of memcpy that crept in). That would be very welcome. But I cannot leave things like this open as bugs here, there would be simply too many feature requests hiding the actual bugs, sorry.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 06 Nov 2022 07:13:37 PM UTC, original submission:  

In 2007 (commit 255d5a74, v1.3.0) the MEMCPY and SMEMCPY macros were introduced
with this description:
    2007-05-09 Simon Goldschmidt
    opt.h, .c/*.h: Included patch #5920: Create define to override C-library
      memcpy. 2 Defines are created: MEMCPY() for normal memcpy, SMEMCPY() for
      situations where some compilers might inline the copy and save a function
      call. Also replaced all calls to memcpy() with calls to (S)MEMCPY().

In 2017 (commit 7cedf7ae) MEMMOVE was also added along the same lines.

A few direct (lowercase) uses have creeped in since. I am working on a project with NO_SYS=1, that should build without a libc available, and I am defining MEMCPY and MEMMOVE to internal reimplementations of these functions. I hit on memcpy() being directly used in ip6addr_ntoa_r (ip6_addr.c) and memmove() in lwip_itoa (def.c). Grepping through the tree reveals a few more (but fortunately not too many). These should probably be replaced with the already existing MEMCPY and MEMMOVE macros.

Additionally it would be good to define and use other string.h macros like MEMCMP and MEMSET. I ran into on memcmp() in eth_addr_cmp (ethernet.h), and memset() in netif_add (netif.c), tcp_alloc (tcp.c), udp_new (udp.c), ip_reass_enqueue_new_datagram (ipv4/ip4_frag.c), ip6_reass (ipv6/ip6_frag.c), dns_send (dns.c), plus a few uses in ipv4/dhcp.c. Finally, I also ran into strlen() and strncmp() in pbuf_strstr (pbuf.c), lwip_strnstr (def.c) as well as strlen() in dns_send (dns.c). It does feel a bit like a game of whack-a-mole, so I am curious how you feel this compares to the existing facilities (MEMCPY/MEMMOVE), and if there are other recommendations for systems where a libc is unavailable.

In all of those cases, it looks like there is an #include <string.h> in the source files. Given this exists solely for those functions above, it'd be nice to avoid it, especially in NO_SYS mode. For what it's worth, on a simple NO_SYS=1 configuration, these 6 (memcmp, memset, strlen, strncmp, plus memcpy/memmove) are enough to avoid string.h altogether.

That, plus atoi() in stdlib.h are the only libc dependencies that I had to workaround for my build. (I wonder if I should file a separate bug for atoi?)

Finally, I am also unsure why these macros live in lwipopts.h rather than cc.h? The distinction between the two feels somewhat blurry, but my understanding is that it may fit better in cc.h.

Faidon Liambotis <paravoid>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by paravoid (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-10-11 goldsimon StatusNone Works For Me
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code