@bu5hm4n When I wrote raw diff what I really mean is the file diff plus metatdata like author, commit subject and message, date, etc

@n3rdopolis Still the same problem. If you can'Ät use git phab or arc please upload the raw git format-patch elsewhere and post the link here.

@devilhorns I see.
@n3rdopolis did you upload the patch by pasting in a raw git diff? Or did your use the git phab or arc tools? We have seen this exact problem with the later variant.
If you can't switch to git phab or arc I would suggest that you send a link to the full git format-patch output so we can apply that one.

@devilhorns, @raster you want this in before the final release?

The comment suggest that its for a clean up case. So maybe we do not have them around to be cleaned up and thus fail.

In a normal ninja test run on my local machine:

With the variable name fixed the tests still fail for me

Already fixed in master

Now would be a good time, as we are in stabilization for 1.25

While the tests build error is gone the different signedness warnings are still there

@eagleeye please give code from raster a try. He will land it shortly and we would like confirmation from your side that the issue is really fixed with it. You stated that you can reproduce this in automated testing. We would like to get that confirmed for the release. Thanks

Its even more than that. When running the test suite I get the following:

Applies now, but shows errors when building:

[1/3252] Compiling C object 'src/bin/eolian/70b716a@@eolian_gen@exe/main.c.o'
../src/bin/eolian/main.c: In function ‘_read_options_from_response_file’:
../src/bin/eolian/main.c:524:22: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
  524 |    for (int i = 0; i < file_length; ++i)
      |                      ^
../src/bin/eolian/main.c:543:22: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
  543 |    for (int i = 0; i < file_length; ++i)
      |                      ^
../src/bin/eolian/main.c:548:23: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
  548 |              while (i < file_length && contents[i] == '\0')
      |                       ^
../src/bin/eolian/main.c:553:20: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
  553 |              if (i >= file_length)
      |                    ^~

This does not apply to latest HEAD anymore, please rebase.

In addition to the duplicated check free() can handle NULL pointers without problem. No need to check for them.

For me, on Linux, this does not fail either. Patch looks sane. @vtorri could you give it another try?

If you want this to land you would need to rebase. It does not apply.

I am landing this, but you should look into fixing your git commit author

Ah, so that was breaking the release builds :-) Thanks

The remaining 30 cxx related Coverity issues are now making up 30% of the open Coverity we still have. It would be great if you could find some timeslots to look at a few of these.

Your commit message is really sparse.

I think its fine to have all the mechanical changes in the patch here as well. It only took me 5 minutes to review even with this size.

We talked about this before. Finally came around to remove it.

I am fine with the patch now. Before it can land it needs a rebase though. Does not apply here.

Limited review as I have no clue on the windows system side. :-)