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.

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

Jump to bottom

Raise the bar to ISO C99 (drop support for C89) #601

Merged
merged 10 commits into from Dec 12, 2022
Merged

Raise the bar to ISO C99 (drop support for C89) #601

merged 10 commits into from Dec 12, 2022

Conversation

alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Dec 2, 2022 •
edited

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 2, 2022 •
edited

I think this is all for C99.

After this is merged, I'll suggest raising the bar even further, to POSIX.1-2001 (#602). That PR for POSIX.1-2001 will be a draft until this one is merged, since it depends on these changes.

@alejandro-colomar alejandro-colomar marked this pull request as ready for review December 2, 2022 16:29
@alejandro-colomar
Copy link
Collaborator Author

Rebased to master.

ikerexxe
ikerexxe reviewed Dec 5, 2022
View reviewed changes
Copy link
Collaborator

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

Apart from a little comment inline, I'd also like to ask you to add the reference to the issue fixed in the commit messages. Something like:

Resolves: https://github.com/shadow-maint/shadow/issues/600

lib/defines.h Show resolved Hide resolved
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 5, 2022 •
edited

Apart from a little comment inline, I'd also like to ask you to add the reference to the issue fixed in the commit messages. Something like:

Resolves: https://github.com/shadow-maint/shadow/issues/600

Yep, I think that would be conventionally Closes:. Although since the issue really covers more than just C99, I think I'll just use Link:. Are you happy with it?

@alejandro-colomar
Assume <stdbool.h> exists
aa5f77d 
ISO C99 requires <stdbool.h>.

Many files in the project already include <stdbool.h> unconditionally,
so it's reasonable to assume that it is always available.

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume <errno.h> exists
8c14c46 
ISO C99 requires <errno.h>.

Many files in the project already include <errno.h> unconditionally,
so it's reasonable to assume that it is always available.

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Remove unused check for <limits.h>
57702b5 
Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume <locale.h> exists
da01b83 
ISO C99 requires <locale.h>.

Other files in the project already include <locale.h> unconditionally,
so it's reasonable to assume that it is always available.

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume fputs(3) exists
624c35d 
ISO C99 requires fputs(3).

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume strerror(3) exists
eff95dc 
ISO C99 requires strerror(3).

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume NULL exists
3780433 
ISO C99 requires NULL.

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 5, 2022 •
edited

Changes:

  • Remove reference to strerror(3) in configure.ac. [Iker]
  • Add Link and Cc to the commit messages.

@alejandro-colomar
Copy link
Collaborator Author

I didn't understand AC_REPLACE_FUNCS(). Now that I've checked it, I've seen there are a few other removals that I can do. Please don't apply yet :)

@alejandro-colomar
Assume rename(2) exists
79e196f 
ISO C99 requires rename(2).

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume snprintf(3) exists
580e37d 
ISO C99 requires snprintf(3).

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Assume strstr(3) exists
0681d93 
ISO C99 requires strstr(3).

Link: <shadow-maint#600>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 5, 2022 •
edited

Okay, now it should be ready.

Changes:

  • Also remove checks for 3 other functions required by C99 (and in some cases even C89).

@ikerexxe
Copy link
Collaborator

ikerexxe commented Dec 5, 2022

Yep, I think that would be conventionally Closes:. Although since the issue really covers more than just C99, I think I'll just use Link:. Are you happy with it?

As long as you reference the issue I'm fine.

LGTM. Thanks for the patches!

@alejandro-colomar
Copy link
Collaborator Author

:-)

@ikerexxe ikerexxe mentioned this pull request Dec 5, 2022
Merged
@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 5, 2022 •
edited

I'm happy to help. And BTW, I learnt a lot with these patches. Just FYI, I started rewriting some of the Linux man-pages about strings, and plan a new string(7) manual page that details all commonly used string handling functions (and some more that I've only seen customly written in high-performance projects such as nginx), and add details when each of them is adequate.

As a starter, I already killed strncpy(3), in the hope that nobody ever uses it again ;)
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man3/strncpy.3#n26

Oh, and it just feels great chopping old code :)

@ikerexxe ikerexxe merged commit d7baafb into shadow-maint:master Dec 12, 2022
@alejandro-colomar
Copy link
Collaborator Author

Great! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikerexxe ikerexxe ikerexxe left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alejandro-colomar @ikerexxe