[Bug 231137] New: fvwm2-2.5.19-7: comparison with string literal
https://bugzilla.novell.com/show_bug.cgi?id=231137 Summary: fvwm2-2.5.19-7: comparison with string literal Product: openSUSE 10.3 Version: unspecified Platform: Other OS/Version: Other Status: NEW Severity: Normal Priority: P5 - None Component: Basesystem AssignedTo: bnc-team-screening@forge.provo.novell.com ReportedBy: dcb314@hotmail.com QAContact: qa@suse.de I just tried to compile package fvwm2-2.5.19-7 The compiler said Flocale.c:1079: warning: comparison with string literal The source code is if (fn != NULL && fn != FLOCALE_FFT_FALLBACK_FONT) but dcb > find ../BUILD/fvwm-* -name \*.h -print | xargs fgrep FLOCALE_FFT_FALLBACK_FONT ./BUILD/fvwm-snap-20061212/libs/defaults.h:#define FLOCALE_FFT_FALLBACK_FONT "MonoSpace" Suggest new code if (fn != NULL && (strcmp( fn, FLOCALE_FFT_FALLBACK_FONT) != 0)) -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 chrubis@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|bnc-team- |lmichnovic@novell.com |screening@forge.provo.novell| |.com | -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 lmichnovic@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED ------- Comment #1 from lmichnovic@novell.com 2007-01-08 07:45 MST ------- Thank you for reporting. Fix applied. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 lmichnovic@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED | ------- Comment #2 from lmichnovic@novell.com 2007-01-10 06:03 MST ------- In this case is the comparison correct. Suggested new code could break functionality. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 lmichnovic@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |INVALID ------- Comment #3 from lmichnovic@novell.com 2007-01-10 06:04 MST ------- Not applying any patch. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 dcb314@hotmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|INVALID | ------- Comment #4 from dcb314@hotmail.com 2007-01-10 11:13 MST ------- (In reply to comment #2)
In this case is the comparison correct.
It looks correct to me. If you have a better suggestion, or can find fault with the suggested patch, then please tell me.
Suggested new code could break functionality.
How ? -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 lmichnovic@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |WONTFIX ------- Comment #5 from lmichnovic@novell.com 2007-01-11 03:13 MST ------- The FLOCALE_FFT_FALLBACK_FONT is static string which is represented AFAIK as static pointer in the heap. And they want to compare if the pointer fn is not pointing on the same place as that static pointer. They don't care what string is there. In some way th fn could be initialized to string "MonoSpace" and will not pointing to FLOCALE_FFT_FALLBACK_FONT. So in this case the patched comparision would give false positive result. The way how it is written in fvwm2 is correct. I have consulted your patch with fvwm2 upstream. They claimed it will break functionality. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 dcb314@hotmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|WONTFIX | ------- Comment #6 from dcb314@hotmail.com 2007-01-11 05:15 MST ------- (In reply to comment #5)
The FLOCALE_FFT_FALLBACK_FONT is static string which is represented AFAIK as static pointer in the heap.
Nearly - static strings are stored in the bss section, not the heap.
And they want to compare if the pointer fn is not pointing on the same place as that static pointer. They don't care what string is there.
This assumes that identical text strings point to the same place. This is undefined in ISO C.
In some way th fn could be initialized to string "MonoSpace" and will not pointing to FLOCALE_FFT_FALLBACK_FONT. So in this case the patched comparision would give false positive result.
I fail to understand how - if fn is pointing at string "MonoSpace", then the result of the strcmp will be non zero. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 ------- Comment #7 from lmichnovic@novell.com 2007-01-11 05:52 MST ------- I've got this reply from upstream: -- The comparison is a valid one. It's there to make sure that the font is freed, unless it's exactly the string literal, since literals can't be freed. If it has the same value as the literal value, but is allocated it still has to be freed. -- To your second question:
I fail to understand how - if fn is pointing at string "MonoSpace", then the result of the strcmp will be non zero.
Please look in the code. fn is initialized few lines above in the mentioned line and let assume that it is initialized to string "MonoSpace". But it is not the fallback font FLOCALE_FFT_FALLBACK_FONT. So comparison (fn != NULL && fn != FLOCALE_FFT_FALLBACK_FONT) is true because fn is not NULL and it is not pointing to fallback font. But comparison (fn != NULL && (strcmp( fn, FLOCALE_FFT_FALLBACK_FONT) !=0)) the fn is not NULL but strcmp will give a matching strings so it differs from previous comparison. And the fn will not be freed. This code works as it was intent. We can ignore the warning from compiler. Satisfied? -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 ------- Comment #8 from dcb314@hotmail.com 2007-01-11 12:53 MST ------- (In reply to comment #7)
Satisfied?
I am now. Thanks for making the extra explanation. I should also mention that there is some talk of making the warning "comparison with string literal" into a fatal build error. There is no defined timescale for this, and I don't even know if it is certain to happen yet. It seems clear that this package would then need some non-trivial rework. It also might be a wise idea to close this bug report. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
https://bugzilla.novell.com/show_bug.cgi?id=231137 lmichnovic@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution| |INVALID ------- Comment #9 from lmichnovic@novell.com 2007-01-12 02:45 MST ------- Closing. -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.
participants (1)
-
bugzilla_noreply＠novell.com