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

check-deps:changed gelf function in __have_libelf.c #1959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JangSoJin
Copy link
Contributor

The currently reflected __have_libelf.c may cause errors during compilation if unused variables(GElf_Ehdr ehdr) are declared.

so i would like to modify it as follows :

#include <gelf.h>
#include <libelf.h>

int main(void)
{
      Elf *elf;

      elf_version(EV_CURRENT);

      /* check that the gelf function */
      elf = elf_begin(0, ELF_C_READ, NULL);
      gelf_checksum(elf);
      elf_end(elf);

      return 0;
}

Thanks,
Fixed : #1869 (gcc -Wall -Werror: libelf: [ OFF ] even when libelf-devel is installed)

@JangSoJin JangSoJin closed this Sep 2, 2024
@vt-alt
Copy link

vt-alt commented Sep 3, 2024

I only wonder why you break formatting of the original file from a tab to 6 spaces.

ps. Ah, the CI reported the same. BTW, you don't need to close PR to fix this, just fix and push -f again to the branch.

@JangSoJin
Copy link
Contributor Author

oh, okay thanks @vt-alt
i forgot to check format in the procedure, sorry.
i’ll fix and push again.

@JangSoJin JangSoJin reopened this Sep 3, 2024
@JangSoJin JangSoJin force-pushed the check-deps_develop branch 3 times, most recently from 9c3c270 to fd2d85b Compare September 3, 2024 15:13
@JangSoJin
Copy link
Contributor Author

[Re-upload]

The currently reflected __have_libelf.c may cause errors during compilation if unused variables(GElf_Ehdr ehdr) are declared.
so i would like to modify it as follows :

#include <gelf.h>
#include <libelf.h>

int main(void)
{
      Elf *elf;

      elf_version(EV_CURRENT);

      /* check that the gelf function */
      elf = elf_begin(0, ELF_C_READ, (Elf *)0);
      gelf_checksum(elf);
      elf_end(elf);

      return 0;
}

FYI, when i tried elf_begin(0, ELF_C_READ, NULL), the result was incorrect, so i changed elf_begin(0, ELF_C_READ, (Elf *)0).
(i don't understand what the difference is between,,)
※ if it needs to be modified, please let me know.

Sorry for the inconvenience!
Thanks,
Fixed : #1869 (gcc -Wall -Werror: libelf: [ OFF ] even when libelf-devel is installed)

@gichoel
Copy link
Contributor

gichoel commented Sep 3, 2024

First, Thank you for your efforts to resolve this issue.

However, There are a few rules to follow to write a good readable commit message, and there are three things you should fix in your commit message:

  1. The first letter of the title, except for the 'type' part, should be capitalized.
  2. The title should be 50 characters or less, but currently exceeds 50 characters, including spaces.
  3. The body should also be 72 characters or less, but there are lines of body that exceed 72 characters.

Could you please make this change?

Even though 'libelf-devel' is installed,
'libelf' is displayed as [OFF] rather than [ON]
when running ./configure.

Fixed : namhyung#1869

Signed-off-by: SoJin Jang <wkdthwls3@naver.com>
@JangSoJin
Copy link
Contributor Author

Thanks, @gichoel
i made the changes based on your feedback, thanks for the support!

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

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

LGTM

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.

gcc -Wall -Werror: libelf: [ OFF ] even when libelf-devel is installed
4 participants