Skip to content
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.

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

Review krb5 patches #17

Open
cristian-vijelie opened this issue Nov 20, 2020 · 1 comment
Open

Review krb5 patches #17

cristian-vijelie opened this issue Nov 20, 2020 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@cristian-vijelie
Copy link

Review the patches on patchwork:
https://patchwork.unikraft.org/bundle/cvijelie/kerberos/

Note: only the 3 support libraries should be reviewed at the moment

@cristian-vijelie cristian-vijelie added the enhancement New feature or request label Nov 20, 2020
@laurbrb
Copy link

laurbrb commented Nov 26, 2020

Makefile.uk

  • it doesn't have the copyright header;
  • the patches directory is missing from the base folder;
  • I think that we should suppress the warnings generated when building lib5crypto, libkrb5support and liberr_com using these flags: -Wno-unused-parameter, -Wno-shadow, -Wno-parentheses, -Wno-implicit-fallthrough, -Wno-missing-field-initializers, -Wno-empty-body, -Wno-suggest-attribute=format.

README.md

  • besides of the typo, I suggest to reformulate the following sentence as follows:
	5 - When adding the libraru in the dependencies list, lib-krb5, the libraries should be like this:
	5 + When adding the library, lib-krb5, to the dependencies list, the libraries should be in this order:

Config.uk

  • typo:
	10 - 	select LIBNEWLIB
	10 + 	select LIBNEWLIBC
  • lib-krb5 also depends on libdl, so we can set libdl too(select LIBPOSIX_LIBDL);
  • I know that the state of this library is 'Work In Progress", but in this version LIBGSSAPI_KRB5 is only declared and never used.

Glue code

  • you can remove include/byteswap.h, since now it is included in newlib/musl-imported/include/.

craciunoiuc pushed a commit that referenced this issue Aug 4, 2022
GitHub-Closes: #17
Signed-off-by: Alexander Jung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants