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

common : Update stb_image.h to latest version #9161

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

arch-btw
Copy link
Contributor

Fixes #7431

See this issue in the original repo for more details: nothings/stb#1642 (comment)

@arch-btw arch-btw changed the title Update stb_image.h to latest version common: Update stb_image.h to latest version Aug 24, 2024
@arch-btw arch-btw changed the title common: Update stb_image.h to latest version common : Update stb_image.h to latest version Aug 24, 2024
@ggerganov
Copy link
Owner

There seems to be some formatting applied to the stb_image.h in this PR. It's better to use the original header from upstream without changes

@arch-btw
Copy link
Contributor Author

Oh yes, I see what you mean!
Is it possible by any chance that the original PR had custom formatting?

I'm just wondering because I checked every commit in the upstream repo starting at 2.28 (and the few commits leading up to it) and non of them match the file that we currently have in llama.cpp.

Then I verified the dev branch of upstream, but it doesn't match our current file either.

I also checked the shasum on this pull request and it matched the latest shasum of upstream:
594c2fe35d49488b4382dbfaec8f98366defca819d916ac95becf3e75f4200b3

Here are the steps that I took for this pull request:

  1. Fork llama.cpp without making changes
  2. Download stb_image.h from upstream
  3. Delete stb_image.h from fork
  4. Add stb_image.h to my fork without making any changes
  5. Submit merge request

Feel free to let me know if I'm misunderstanding or how to proceed :)
Thank you!

Copy link
Owner

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

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

Is it possible by any chance that the original PR had custom formatting?

Ah, you are right. I got confused.

@ggerganov
Copy link
Owner

Need to exclude stb_image.h from the editor config CI before merging

@ggerganov ggerganov merged commit ad76569 into ggerganov:master Aug 27, 2024
52 checks passed
dsx1986 pushed a commit to dsx1986/llama.cpp that referenced this pull request Oct 29, 2024
* Update stb_image.h to latest version

Fixes ggerganov#7431

* Update .ecrc
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* Update stb_image.h to latest version

Fixes ggerganov#7431

* Update .ecrc
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* Update stb_image.h to latest version

Fixes ggerganov#7431

* Update .ecrc
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.

Potential string overflow in stbi_parse_png_file function
2 participants