unikraft / lib-newlib

Unikraft port of newlib, a C standard library
Other
5 stars 23 forks source link

Remove poll/select and eventfd stubs #23

Closed marcrittinghaus closed 2 years ago

marcrittinghaus commented 2 years ago

This commit removes the stubs for functions provided by the new posix-socket and posix-event libraries.

This PR is part of a larger group of dependent PRs which must be merged in the following order:

  1. https://github.com/unikraft/lib-newlib/pull/23 (this PR)
  2. https://github.com/unikraft/unikraft/pull/484
  3. https://github.com/unikraft/unikraft/pull/65
  4. https://github.com/unikraft/lib-lwip/pull/17
  5. https://github.com/unikraft/unikraft/pull/485

⚠️ Without having merged all of these PRs, network is no longer functional. All PRs should thus be checked and reviewed first and then merged in quick succession⚠️

craciunoiuc commented 2 years ago

All good here 👍

Waiting for more approval before adding the tag

razvand commented 2 years ago

Hi, @marcrittinghaus, this is my first pass. It seems OK, I have some questions:

Why were the STD..._FILENO macro defined in this file.c file in the first place?

What was the initial purpose of including the uk/plat/console.h and uk/print.h header files?

Does it make sense to still have the stdlib.h header file included in file.c?

marcrittinghaus commented 2 years ago

Hi, @marcrittinghaus, this is my first pass. It seems OK, I have some questions: Why were the STD..._FILENO macro defined in this file.c file in the first place? What was the initial purpose of including the uk/plat/console.h and uk/print.h header files?

@razvand If you look at the history of the file you see that I once required all these definitions and headers. Over time more and more stubs were removed but these definitions remained.

Does it make sense to still have the stdlib.h header file included in file.c?

Looking at the man page realpath seems to be defined in stdlib.h although this is not the case in newlib. So technically we could remove it. I just kept it due to conformance with the man page.

razvand commented 2 years ago

@marcrittinghaus , all is OK. @craciunoiuc , please approve this PR and then I'll approve it as well. I'll be careful to do the approval in the order signaled by @marcrittinghaus .