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

Quality of life readability improvements #360

Merged
merged 3 commits into from
Dec 21, 2021

Conversation

fzakaria
Copy link
Contributor

Series of changes that merely change some function names and introduce a header file to help make the code more legible.

  • introduce patchelf.h header file
  • rename findSection to findSectionHeader to better distinguish sections from section-headers
  • rename findSection2 to tryFindSection to better distinguish it's optional
  • rename findSection3 to findSectionIndex

Added a header file to make things easier to navigate.

Renamed findSection3 -> getSectionIndex since thats more sensible to
read.

Renamed findSection -> findSectionHeader to better distinguish sections
from headers.
src/patchelf.cc Show resolved Hide resolved
@fzakaria
Copy link
Contributor Author

Hmm.
I see it adding the current directory to the g++ call

make[1]: Entering directory '/gha/patchelf-0.14.3/src'
g++ -DPACKAGE_NAME=\"patchelf\" -DPACKAGE_TARNAME=\"patchelf\" -DPACKAGE_VERSION=\"0.14.3\" -DPACKAGE_STRING=\"patchelf\ 0.14.3\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"patchelf\" -DVERSION=\"0.14.3\" -I.    -Wall -std=c++17 -D_FILE_OFFSET_BITS=64     -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wformat -Werror=format-security -O2 -static -MT patchelf.o -MD -MP -MF .deps/patchelf.Tpo -c -o patchelf.o patchelf.cc

You can see -I.

@fzakaria
Copy link
Contributor Author

@Mic92 i started alpine directly and entered the nix-shell and it builds.

/app # cat /etc/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.15.0
PRETTY_NAME="Alpine Linux v3.15"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"

/app # ls src/patchelf.h 
src/patchelf.h

/app # make
Making all in src
make[1]: Entering directory '/app/src'
g++ -DPACKAGE_NAME=\"patchelf\" -DPACKAGE_TARNAME=\"patchelf\" -DPACKAGE_VERSION=\"0.14.3\" -DPACKAGE_STRING=\"patchelf\ 0.14.3\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"patchelf\" -DVERSION=\"0.14.3\" -I.    -Wall -std=c++17 -D_FILE_OFFSET_BITS=64     -g -O2 -MT patchelf.o -MD -MP -MF .deps/patchelf.Tpo -c -o patchelf.o patchelf.cc
mv -f .deps/patchelf.Tpo .deps/patchelf.Po
g++ -Wall -std=c++17 -D_FILE_OFFSET_BITS=64     -g -O2   -o patchelf patchelf.o  
make[1]: Leaving directory '/app/src'
Making all in tests

Not sure the pull-request action to download the distribution. Could it be not including patchelf.h somehow ?

@@ -19,4 +19,4 @@ endif

bin_PROGRAMS = patchelf

patchelf_SOURCES = patchelf.cc elf.h
patchelf_SOURCES = patchelf.cc elf.h patchelf.h
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Mic92 found the reason!
I am not very familiar with autotools, so this is all new to me :)

@fzakaria fzakaria requested a review from Mic92 December 21, 2021 18:30
@Mic92 Mic92 merged commit c172ce6 into NixOS:master Dec 21, 2021
@Mic92
Copy link
Member

Mic92 commented Dec 21, 2021

Thanks!

fzakaria added a commit to fzakaria/patchelf that referenced this pull request Dec 22, 2021
commit c172ce6
Merge: b73dbc1 4604393
Author: Jörg Thalheim <Mic92@users.noreply.github.com>
Date:   Tue Dec 21 19:47:20 2021 +0000

    Merge pull request NixOS#360 from fzakaria/faridzakaria/fix-add-replace

    Quality of life readability improvements

commit 4604393
Author: Farid Zakaria <farid.m.zakaria@gmail.com>
Date:   Mon Dec 20 15:04:57 2021 -0800

    Renamed findSection2 to tryFindSectionHeader

commit 1071237
Author: Farid Zakaria <farid.m.zakaria@gmail.com>
Date:   Mon Dec 20 14:59:15 2021 -0800

    Renamed findSection3 to getSectionIndex

commit 05e8f67
Author: Farid Zakaria <farid.m.zakaria@gmail.com>
Date:   Mon Dec 20 14:54:37 2021 -0800

    Added patchelf.h

    Added a header file to make things easier to navigate.

    Renamed findSection3 -> getSectionIndex since thats more sensible to
    read.

    Renamed findSection -> findSectionHeader to better distinguish sections
    from headers.
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.

None yet

2 participants