Skip to content

Fix conflicts to support Newlib4.4 and pthread-embedded #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
Open

Fix conflicts to support Newlib4.4 and pthread-embedded #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ef6ce90
Add BIG LITTLE endian macros for x64
SalarSamani Jul 28, 2025
1138d35
Add BIG LITTLE endian support for x64 and ARM64
SalarSamani Jul 28, 2025
003435e
Delete sendfile.c
SalarSamani Aug 10, 2025
647762a
delete sendfile.c from Makefile.uk
SalarSamani Aug 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile.uk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ LIBLWIP_CFLAGS-y += -Wno-type-limits -Wno-unused-parameter
LIBLWIP_CFLAGS-$(call have_clang) += -Wno-macro-redefined
LIBLWIP_CFLAGS-$(CONFIG_LWIP_DEBUG) += -DUK_DEBUG
LIBLWIP_CFLAGS-y += -D__IN_LIBLWIP__
LIBLWIP_CFLAGS-y += -DWORKAROUND_NEWLIB
Copy link
Member

@craciunoiuc craciunoiuc Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could have a better name?

Copy link
Author

@SalarSamani SalarSamani Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is suggested by chatGPT :D , if you think it is a bad suggestion I can change it ;)

Copy link
Member

@StefanJum StefanJum Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually needed here? I don't see any changes related to this.

LIBLWIP_CXXFLAGS-y += -D__IN_LIBLWIP__

################################################################################
Expand All @@ -94,7 +95,6 @@ LIBLWIP_SRCS-$(CONFIG_LWIP_THREADS) += $(LIBLWIP_BASE)/mailbox.c|unikraft
LIBLWIP_SRCS-$(CONFIG_LWIP_THREADS) += $(LIBLWIP_BASE)/threads.c|unikraft
LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/init.c|unikraft
LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/time.c|unikraft
LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/sendfile.c|unikraft
Copy link
Member

@StefanJum StefanJum Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not needed anymore? Is it the same for musl builds?

LIBLWIP_SRCS-$(CONFIG_LWIP_SOCKET) += $(LIBLWIP_BASE)/sockets.c|unikraft
LIBLWIP_SOCKETS_FLAGS-$(call gcc_version_ge,8,0) += -Wno-cast-function-type
LIBLWIP_SRCS-y += $(LIBLWIP_EXTRACTED)/core/init.c
Expand Down
7 changes: 7 additions & 0 deletions include/netinet/tcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ typedef u_long tcp_seq;
* TCP header.
* Per RFC 793, September, 1981.
*/

#include <uk/config.h>

#if ((defined CONFIG_ARCH_X86_64) || (defined CONFIG_ARCH_ARM_64))
#include <endian.h>
#endif
Comment on lines +47 to +49
Copy link
Member

@craciunoiuc craciunoiuc Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this for x86 and arm only? I know we don't support the others that well, but do you believe that this would not work with RISCV for example?

Copy link
Author

@SalarSamani SalarSamani Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code works for any 64-bit little-endian platform. I added this macro because some libraries, like lwIP, depend on the endianness of the system. When we add support for a new architecture, we must also add it here. If we don’t, we will get an error. That error is on purpose, it shows us that this code depends on the platform, and we need to update it (if it is needed for new platform). In this way, the errors help us remember what to consider when porting to new platforms.

Copy link
Member

@StefanJum StefanJum Aug 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to not error on new architectures. It's expected for lwip to depend on endianess, and the point of endian.h is to make it transparent. So, I would remove the ifdefs.


struct tcphdr {
u_short th_sport; /* source port */
u_short th_dport; /* destination port */
Expand Down
40 changes: 0 additions & 40 deletions sendfile.c
View file
Open in desktop

This file was deleted.