Function get_args from bw-rle.c and pix-rle. is put in file util.c
Updated the patch. Added necessary comments for util.c and util.h and deleted unnecessary headers
This patch attempts to ignore non-existant file util1.c - does it require another patch?
Patch was updated. This patch must go together with patch #3534958, because
Makefile.am and CMakeLists.txt are the same for them.
The build system changes refer to a util1.h and util1.c -- which aren't included. There should not be both util.c/util.h and util1.c/util1.h duplicates. Disambiguating files by adding a number is not a good practice. There shouldn't be more than a single "utility" interface common to the various tools.
Now the patch is updated and doesn't depend on other one. Files are renamed
That's looking a lot better, Ksenija. The only issue I now see is that you moved several static globals into the new header. That means any file that includes that new rle_util.h header will get a copy of those variables. That's not a major issue, but is bad design and would be even more work to undo.
The proper fix while refactoring the new get_args() function is to make it take the variables that it modifies as parameter(s) to the new get_args() function. That way, you can eliminate the static globals altogether, and the two applications will have their own copies of that data (not from a header).
Sean, thanks. I've updated patch according to your suggestions
Oof, not quite. C passes by value, not by reference. You added all of the function arguments, but they're the wrong type. You need to pass pointers to them. That goes for all of those variables (e.g., &file_width) otherwise the value set inside get_args() will not be accessible after that function returns.
They all need to be pointers to those variables being referenced. You should be able to observe the bugs it would introduce by running the tool with command-line arguments accessing any of those variables.
Patch's updated according to the suggestions. Is it an expected result?
That is better, but still not right. You didn't convert all of the pointers and this patch unmodified would have introduced two new bugs. Ksenija, you have to test your changes!
Testing tools after you make changes like this shouldn't be an afterthought... and it's not optional. Especially since you've been making so many mistakes, you gotta test your patches. You would have quickly and easily discovered that the output file was no longer working.
I made the changes necessary, tested, and applied them in r52142 but please, PLEASE test your other patches.
Sign up for the SourceForge newsletter:
You seem to have CSS turned off.
Please don't fill out this field.