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

Move C fallback functions into arch/generic [Part 2] #1631

Merged
merged 5 commits into from
Jan 19, 2024
Merged

Conversation

Dead2
Copy link
Member

@Dead2 Dead2 commented Jan 4, 2024

This builds upon PR #1630, and needs to be rebased once that is in.

Moves these C functions into arch/generic

  • Chunkset and related
  • inflate_fast
  • insert_string and update_hash
  • slide_hash
  • compare256 and longest_match

Also moves include files out of match_tpl.h to simplify dependency handling and avoid attempting to parse the includes more times than needed.

@Dead2 Dead2 added the cleanup Improving maintainability or removing code. label Jan 4, 2024
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (06895bc) 83.11% compared to head (062149b) 83.12%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1631   +/-   ##
========================================
  Coverage    83.11%   83.12%           
========================================
  Files          135      135           
  Lines        10898    10898           
  Branches      2817     2817           
========================================
+ Hits          9058     9059    +1     
  Misses        1130     1130           
+ Partials       710      709    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@nmoinvaz nmoinvaz left a comment

Choose a reason for hiding this comment

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

LGTM

slide_hash_c.lo: $(SRCDIR)/slide_hash_c.c $(SRCTOP)/zbuild.h $(SRCTOP)/deflate.h
$(CC) $(SFLAGS) $(INCLUDES) -c -o $@ $(SRCDIR)/slide_hash_c.c


Copy link
Member

Choose a reason for hiding this comment

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

Is two new lines here intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was not intentionally added in that commit, but I had intended to keep 2 empty lines above and below that list from the beginning, to provide a bit more separation in the file. IMO it looks cleaner, but that is just my personal preference.

Copy link
Member

Choose a reason for hiding this comment

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

Ok.

win32/Makefile.a64 Outdated Show resolved Hide resolved
win32/Makefile.arm Outdated Show resolved Hide resolved
@Dead2 Dead2 force-pushed the no-c-fallback-p2 branch 3 times, most recently from ce8ecc4 to ed65e39 Compare January 16, 2024 17:52
@Dead2 Dead2 merged commit 86250d4 into develop Jan 19, 2024
271 checks passed
@Dead2 Dead2 deleted the no-c-fallback-p2 branch February 22, 2024 18:57
@Dead2 Dead2 mentioned this pull request May 30, 2024
@Dead2 Dead2 mentioned this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Improving maintainability or removing code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants