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

Solved some compilation warnings #281

Closed
wants to merge 2 commits into from

Conversation

RaduMantu
Copy link
Contributor

@RaduMantu RaduMantu commented Sep 4, 2021

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.pl on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): x86_64
  • Platform(s): kvm
  • Application(s): N/A

Description of changes

Fixed 4 compilation warnings for core libraries:

  • returning uninitialized value in fputc()
  • missing return statement in sync()
  • added cast to unsigned long for address in ioctl()
  • replaced implicit declaration of getpid() when libc-style stubs are not generated

@nderjung nderjung linked an issue Sep 5, 2021 that may be closed by this pull request
@nderjung nderjung added kind/maintenance usoc21 Unikraft Summer of Code 2021 labels Sep 5, 2021
@nderjung nderjung added lib/nolibc Only neccessary subset of libc functionality lib/uksignal lib/vfscore VFS Core Interface labels Sep 28, 2021
Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Hi @RaduMantu! Thanks for the PR and great work again at USoC21!

To get this PR ready to be merged, please can you adjust each commit message so that it:

  1. each commit has a short title, starting with the library prefix, e.g. lib/vfscore: ;
  2. that the line-length of each commit message is no longer than 75 chars;
  3. put the full description within the commit message after the title.

Thanks!

@RaduMantu
Copy link
Contributor Author

Hi @nderjung,

Sorry for the long delay in dealing with these issues.
I've addressed the problems that you pointed out. My fault for not using checkpatch.pl beforehand.

I'll take a look over the other pending PR tomorrow at the latest.

Thanks for taking the time!

@razvand razvand added this to the v0.6 - Dione milestone Nov 17, 2021
Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

Hey @RaduMantu,

All things look good. I was able to reproduce the warnings and fix them with your commits.

The only problem is that the second dcb5db6 and the third 358e27d commit were fixed in the meantime by @skuenzer in the same fashion.
As such, there is a merge conflict. Simply removing the second and third conflict should fix the problem.
After you do this, I will add the tag and it will get merged.

Sorry your patches weren't accepted sooner. I think people just got caught up with other stuff 🙁

Cezar

Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
Replaced implicit declaration of getpid() (when no libc-style wrappers
are generated) with uk_syscall_r_getpid().

Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
@RaduMantu
Copy link
Contributor Author

Hi @craciunoiuc ,

Thanks for the feedback.

Simply removing the second and third conflict should fix the problem.

I removed them both. Should not be any more conflicts.

Sorry your patches weren't accepted sooner. I think people just got caught up with other stuff slightly_frowning_face

It's ok. These are pretty small changes and I get they aren't a priority. Also, I could have responded quicker to change requests :/

I also looked over your comments on the other PR and will address them first thing tomorrow morning.

@craciunoiuc
Copy link
Member

Hey @RaduMantu,

All good now!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@gmail.com

Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

Hey @RaduMantu,

All good now!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@gmail.com

@unikraft-bot
Copy link
Member

Checkpatch passed

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request and it all looks good!

SHA commit checkpatch
02a5425 lib/nolibc: Solved use of possibly uninitialized variable
779e572 lib/uksignal: Solved implicit declaration of getpid()

@razvand razvand removed request for a team and nderjung November 27, 2021 09:24
@razvand razvand assigned nderjung and unassigned hlef Nov 27, 2021
Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

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

All good now!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@gmail.com

Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Thanks! 🐒

Approved-by: Alexander Jung a.jung@lancs.ac.uk

unikraft-bot pushed a commit that referenced this pull request Nov 27, 2021
Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com>
Approved-by: Alexander Jung <a.jung@lancs.ac.uk>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Pull-Request: #281
unikraft-bot pushed a commit that referenced this pull request Nov 27, 2021
Replaced implicit declaration of getpid() (when no libc-style wrappers
are generated) with uk_syscall_r_getpid().

Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com>
Approved-by: Alexander Jung <a.jung@lancs.ac.uk>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Pull-Request: #281
@unikraft-bot unikraft-bot added the ci/merged Merged by CI label Nov 27, 2021
razvand pushed a commit to unikraft-upb/unikraft that referenced this pull request Dec 18, 2021
Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com>
Approved-by: Alexander Jung <a.jung@lancs.ac.uk>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Pull-Request: unikraft#281
razvand pushed a commit to unikraft-upb/unikraft that referenced this pull request Dec 18, 2021
Replaced implicit declaration of getpid() (when no libc-style wrappers
are generated) with uk_syscall_r_getpid().

Signed-off-by: Radu Mantu <andru.mantu@gmail.com>
Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@gmail.com>
Approved-by: Alexander Jung <a.jung@lancs.ac.uk>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Pull-Request: unikraft#281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/merged Merged by CI kind/maintenance lib/nolibc Only neccessary subset of libc functionality lib/uksignal lib/vfscore VFS Core Interface usoc21 Unikraft Summer of Code 2021
Projects
Status: Done!
Status: merged
Development

Successfully merging this pull request may close these issues.

Clean up warnings from core build
6 participants