We hold BRL-CAD to a rather high standard of code quality. We compile with nearly every useful warning that GCC is capable of producing and consider all warnings as errors that stop compilation. This is done because many/most warnings are "code smells" that are eventually usually costly to ignore over time.
This task involves adding the -Wwrite-strings warning to our compilation and fixing the issues that result. You can add the flag by editing misc/CMake/BRLCAD_CompilerFlags.cmake
Make sure you compile cleanly before beginning. Make sure you're using a fresh SVN checkout. See http://brlcad.org/wiki/Compiling for help.
Protip: run this to save all errors to a file so you can work on them in bulk: make -k 21 | tee build.log
Note that nearly all the errors that will result are trivially fixed by adding "const" to a struct declaration. Submit you work as a single patch file, see http://brlcad.org/wiki/Patches
| File name/URL | File size | Date submitted | |
|---|---|---|---|
| const_char_fix.patch | 303.4 KB | January 03 2013 14:56 UTC |
I would like to work on this task.
This task has been assigned to javamonn. You have 48 hours to complete this task, good luck!
So, just to be sure I'm looking at the right errors here, I've added the following to the BRLCAD_CompilerFlags.cmake file:
brlcad_check_c_flag(Wwrite-strings) and brlcad_check_cxx_flag(Wwrite-strings)
and the compilation results in a fair amount of :
error: initialization discards ‘const’ qualifier from pointer target type [-Werror]
These are the errors I should fix, right? I'm just checking because I almost expected to see errors with [-Wwrite-strings] as the last part, since -Werror is its own seperate flag.
Thanks,
Daniel
I just removed the brlcad_check_c_flag(Wwrite_strings) to check myself and these errors still appear, so I'm guessing these are not what I should be fixing (even thought they look to be caused by a similar problem). How exactly do I go about adding the wwrite_strings flag, since it appears there is more to it than adding the lines I already have?
Thanks for the help,
Daniel
So, now I just feel stupid. I just realized I had those two lines of code in two sepearte places in that file. When I removed them both, the errors dissapeared. So it looks like I did set that flag correctly after all, haha. I'll get to work now.
Thanks,
Daniel
The claim on this task has been removed, someone else can claim it now.
Sounds like you have/had the right idea before you unclaimed. Instead of removing lines of code, though, you'll need to propagate 'const' in a lot of places.
I would like to work on this task.
This task has been assigned to gcking. You have 48 hours to complete this task, good luck!
Melange has detected that the initial deadline has passed and it has set the task status to ActionNeeded. The student has 24 hours to submit the work before the task is reopened and sent back to the pool for other students to claim.
Melange has detected that the final deadline has passed and it has reopened the task.
I would like to work on this task.
Can I reclaim the task and upload my work?
This task has been assigned to gcking. You have 48 hours to complete this task, good luck!
The task has been reassigned to you and you can now upload your work for review.
Good luck..!!
The work on this task is ready to be reviewed.
Melange has detected that the deadline has passed and no more work can be submitted. The submitted work should be reviewed.
The deadline of the task has been extended with 1 days and 0 hours.
One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.
Your patch by large looks great but there is a problem that prevents the patch from being used. Casting string literals to (char *) is no good.
That tells the compiler that it's okay to allow that memory to be edited, and that's generally a bad thing. That's why the compiler is warning in the first place. The "fix" is usually to make the recipient const. If the setee really is needing to edit the string, then we need to know about those cases an review them on a case-by-case basis to see what's going on.
We can create a follow-up task so you can keep working on this if you've worked more than a few hours already (i.e., scope is off). How long did you work on this?
after my first compilation, I realized it is a big modification. I had spent over 72 hours on it. in many places, if I changed the string literals to be constant variable instead of cast to (char *), then it would have impact on the signature of some invoking function, which will propagate to other modules. It definitely needs more time to do analysis.
The time estimate I'm looking for is actual time typing in an editor, not how long including food breaks and sleeping.. ;) A better estimate perhaps is how long do you think it would take you to sit down and retype everything you just did for this patch if you had to do it again? Would that be 3 non-stop hours of typing? 6 hours? 12 hours?
Either way, go ahead and mark as complete. We can try to create follow-up task before GCI ends to break the task up in chunks.
The deadline of the task has been extended with 0 days and 12 hours.
it is my mistake that gave you the incorrect working hours, because when task is assigned, it is based on calendar days, I was confused by that. but I will try my best to work on this.
The work on this task is ready to be reviewed.
Melange has detected that the deadline has passed and no more work can be submitted. The submitted work should be reviewed.
Congratulations, this task has been completed successfully.
gcking, How long do you think this patch would take if you had to do it again? How many hours of typing?
Sorry for the late reply, I just finished some school work. I guess 10 hours should be ok.