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

Axe automated header generation. #70

Merged
merged 3 commits into from
Sep 8, 2020
Merged

Conversation

zevweiss
Copy link
Contributor

@zevweiss zevweiss commented Sep 2, 2020

Reasoning:

  • implementation was unsound -- broke down when I added a fairly
    basic macro definition expanding to a struct initializer in a *.c
    file.

  • made it way too easy (e.g. via otherwise totally innocuous git
    commands) to end up with timestamps such that it always ran
    MakeHeader.py but never used its output, leading to overbuild noise
    when running what should be a null 'make'.

  • but mostly: it's just an awkward way of dealing with C code.

[Addresses #9.]

@bertwesarg
Copy link
Contributor

Please remove extern from all pure function declarations in the headers. They are superfluous.

Comment on lines -61 to +57
myhtopplatsources = linux/Platform.c linux/IOPriorityPanel.c linux/IOPriority.c \
myhtopplatsources = linux/Platform.c linux/IOPriorityPanel.c \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was linux/IOPriority.c removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because aside from being a source from which the corresponding header was generated, it included nothing of actual substance -- an enum declaration, a few macros, and a typedef (zero variable or function definitions).

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, missed, that the .c file was removed in reallity too.

@zevweiss
Copy link
Contributor Author

zevweiss commented Sep 2, 2020

Agreed re: extern; bulk removal performed in 822154a.

@bertwesarg
Copy link
Contributor

The extra targets in Makefile.am should now be removed too:

.PHONY: htop-headers clean-htop-headers

htop-headers: $(myhtopheaders) $(all_platform_headers)

clean-htop-headers:
	-rm -f $(myhtopheaders) $(all_platform_headers)

Maybe the all_platform_headers variable too

@zevweiss
Copy link
Contributor Author

zevweiss commented Sep 2, 2020

Ah, good point -- and also SUFFIXES and BUILT_SOURCES I believe.

Reasoning:
 - implementation was unsound -- broke down when I added a fairly
   basic macro definition expanding to a struct initializer in a *.c
   file.

 - made it way too easy (e.g. via otherwise totally innocuous git
   commands) to end up with timestamps such that it always ran
   MakeHeader.py but never used its output, leading to overbuild noise
   when running what should be a null 'make'.

 - but mostly: it's just an awkward way of dealing with C code.
Applied via:

  $ find * -name '*.h' -exec sed -i -r 's/^extern (.+\()/\1/;' {} +

Suggested-by: Bert Wesarg <[email protected]>
@zevweiss
Copy link
Contributor Author

zevweiss commented Sep 3, 2020

Rebased to resolve merge conflicts; also added a tiny commit to remove a duplicate declaration introduced in #86.

@natoscott natoscott merged commit eede79b into htop-dev:master Sep 8, 2020
@zevweiss zevweiss deleted the noheadergen branch September 8, 2020 07:38
hugomg added a commit to hugomg/htop that referenced this pull request Sep 12, 2020
PR htop-dev#70 got rid of the infrastructure for generating
header files, but it left behind some code duplication.

Some of cases are things that belong in the header file and don't need
to be reapeated in the C file. Other cases are things that belong in the
C file and don't need to be in the header file.

In this commit I tried to fix most of these that I could find. When
given a choice I preferred keeping things out of the header file, unless
they were being used by someone else.
hugomg added a commit to hugomg/htop that referenced this pull request Sep 12, 2020
PR htop-dev#70 got rid of the infrastructure for generating header
files, but it left behind some code duplication.

Some of cases are things that belong in the header file and don't need
to be repeated in the C file. Other cases are things that belong in the
C file and don't need to be in the header file.

In this commit I tried to fix all of these that I could find. When given
a choice I preferred keeping things out of the header file, unless they
were being used by someone else.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants