Bug 526049 - metacity-2.22 has poor programming practices
metacity-2.22 has poor programming practices
Status: RESOLVED FIXED
Product: metacity
Classification: Other
Component: general
2.22.x
Other All
: Normal minor
: ---
Assigned To: Metacity maintainers list
Metacity maintainers list
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2008-04-03 20:42 UTC by Sandro Bonazzola
Modified: 2016-02-07 23:37 UTC (History)
3 users (show)

See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
Eliminate the delete.c warning (689 bytes, patch)
2008-04-30 21:06 UTC, Matt Kraai
committed Details | Diff | Review

Description Sandro Bonazzola 2008-04-03 20:42:38 UTC
Please describe the problem:
metacity-2.22 has poor programming practices which may compile fine but exhibit random runtime failures.

metacity-mag.c:132: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/iconcache.c:242: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/iconcache.c:481: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/xprops.c:1054: warning: dereferencing type-punned pointer will break strict-aliasing rules

ui/preview-widget.c:487: warning: implicit declaration of function 'sqrt'
ui/preview-widget.c:493: warning: implicit declaration of function 'floor'

ui/preview-widget.c:487: warning: incompatible implicit declaration of built-in function 'sqrt'
ui/preview-widget.c:493: warning: incompatible implicit declaration of built-in function 'floor'
ui/preview-widget.c:506: warning: incompatible implicit declaration of built-in function 'sqrt'
ui/preview-widget.c:511: warning: incompatible implicit declaration of built-in function 'floor'
ui/preview-widget.c:524: warning: incompatible implicit declaration of built-in function 'sqrt'
ui/preview-widget.c:529: warning: incompatible implicit declaration of built-in function 'floor'
ui/preview-widget.c:542: warning: incompatible implicit declaration of built-in function 'sqrt'
ui/preview-widget.c:547: warning: incompatible implicit declaration of built-in function 'floor'


Steps to reproduce:
1. build metacity-2.22 package
2. 
3. 


Actual results:
gcc warn about incompatible declaration of functions and strict-aliasing rules breacking

Expected results:
No implicit / incompatible declaration warning because the source should have
all the declarations it needs.
No strict-aliasing rules breacking.

Does this happen every time?
yes

Other information:
The package seems to work fine if compiled with -fno-strict-aliasing but as a good programming practice, please fix the missing / incompatible declaration and strict-aliasing rules breackage in further releases.
Comment 1 Hubert Figuiere (:hub) 2008-04-03 20:54:34 UTC
you could have provided a patch since everything sounds so trivial.
Comment 2 Sandro Bonazzola 2008-04-06 09:47:06 UTC
(In reply to comment #1)
> you could have provided a patch since everything sounds so trivial.

Thefollowing patch solve:
ui/preview-widget.c:487: warning: implicit declaration of function 'sqrt'
ui/preview-widget.c:493: warning: implicit declaration of function 'floor'

ui/preview-widget.c:487: warning: incompatible implicit declaration of built-in
function 'sqrt'
ui/preview-widget.c:493: warning: incompatible implicit declaration of built-in
function 'floor'
ui/preview-widget.c:506: warning: incompatible implicit declaration of built-in
function 'sqrt'
ui/preview-widget.c:511: warning: incompatible implicit declaration of built-in
function 'floor'
ui/preview-widget.c:524: warning: incompatible implicit declaration of built-in
function 'sqrt'
ui/preview-widget.c:529: warning: incompatible implicit declaration of built-in
function 'floor'
ui/preview-widget.c:542: warning: incompatible implicit declaration of built-in
function 'sqrt'
ui/preview-widget.c:547: warning: incompatible implicit declaration of built-in
function 'floor'

patch:


--- metacity-2.22.0.original/src/ui/preview-widget.c    2008-04-06 11:33:53.653095325 +0200
+++ metacity-2.22.0/src/ui/preview-widget.c     2008-04-06 11:37:53.001095068 +0200
@@ -22,6 +22,7 @@
  */

 #include "preview-widget.h"
+#include <math.h>

 static void     meta_preview_class_init    (MetaPreviewClass *klass);
 static void     meta_preview_init          (MetaPreview      *preview);
Comment 3 Thomas Thurman 2008-04-06 19:01:54 UTC
Oh, I think this is all fixed in trunk, isn't it?  We should just backport it all.

(Some of the downstream maintainers appear to have taken a lot of these fixes out again, but I'm not sure why.)
Comment 4 Sandro Bonazzola 2008-04-07 18:47:12 UTC
(In reply to comment #3)
> Oh, I think this is all fixed in trunk, isn't it?  We should just backport it
> all.
> 
> (Some of the downstream maintainers appear to have taken a lot of these fixes
> out again, but I'm not sure why.)

missing declarations are fixed in trunk rev. 3676. This revision (3676) still have type punned pointers that actually cause crashes:

metacity-mag.c: In function ‘grab_area_at_mouse’:
metacity-mag.c:135: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/iconcache.c: In function ‘read_rgb_icon’:
core/iconcache.c:242: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/iconcache.c: In function ‘get_kwm_win_icon’:
core/iconcache.c:481: warning: dereferencing type-punned pointer will break strict-aliasing rules
core/xprops.c: In function ‘meta_prop_get_values’:
core/xprops.c:1054: warning: dereferencing type-punned pointer will break strict-aliasing rules

and some probably not so alarming warning:

core/delete.c: In function ‘io_from_ping_dialog’:
core/delete.c:249: warning: passing argument 3 of ‘g_io_channel_read_to_end’ from incompatible pointer type
core/xprops.c: In function ‘wm_hints_from_results’:
core/xprops.c:729: warning: signed and unsigned type in conditional expression
core/xprops.c:732: warning: signed and unsigned type in conditional expression
core/xprops.c:733: warning: signed and unsigned type in conditional expression
core/xprops.c: In function ‘size_hints_from_results’:
core/xprops.c:853: warning: signed and unsigned type in conditional expression
core/xprops.c:854: warning: signed and unsigned type in conditional expression
core/xprops.c:855: warning: signed and unsigned type in conditional expression
core/xprops.c:856: warning: signed and unsigned type in conditional expression
core/xprops.c:857: warning: signed and unsigned type in conditional expression
core/xprops.c:858: warning: signed and unsigned type in conditional expression
core/xprops.c:859: warning: signed and unsigned type in conditional expression
core/xprops.c:860: warning: signed and unsigned type in conditional expression
core/xprops.c:861: warning: signed and unsigned type in conditional expression
core/xprops.c:862: warning: signed and unsigned type in conditional expression
core/xprops.c:863: warning: signed and unsigned type in conditional expression
core/xprops.c:864: warning: signed and unsigned type in conditional expression
core/xprops.c:865: warning: signed and unsigned type in conditional expression
core/xprops.c:866: warning: signed and unsigned type in conditional expression
core/xprops.c:871: warning: signed and unsigned type in conditional expression
core/xprops.c:872: warning: signed and unsigned type in conditional expression
core/xprops.c:873: warning: signed and unsigned type in conditional expression


Comment 5 Matt Kraai 2008-04-30 21:06:48 UTC
Created attachment 110190 [details] [review]
Eliminate the delete.c warning

I think that the attached patch fixes the warning in delete.c by changing the type of len to gsize.
Comment 6 Thomas Thurman 2008-05-03 17:13:09 UTC
Matt:  Thanks, committed.
http://svn.gnome.org/viewvc/metacity?rev=3703&view=rev

Note You need to log in before you can comment on or make changes to this bug.