Re: Bug: Number type warning in help.c
Ok, so I turned off digest mode. In the mean time, excuse the cut and paste below:
This patch doesn't really fix the problem, it only masks the compiler warning.
If gtk_window_set_default_size is expecting gints, then sp_document_width should return a gint instead of a gdouble and the document structure should store its width and height as gint. I don't have the source accessible to me right now, so I can't see how it is defined.
If a width was defined that exceeded INT_MAX, then when the gdouble was cast to a gint, data would be lost. A user would see a windo much smaller than they exected being displayed.
Now, the other way round is fine, you can cast away if you're going from a gint TO a gdouble.
Rob,
From: Jon A. Cruz <jon@...51...> Bug: Number type warning in help.c 2003-11-12 10:01
Here's a patch for bug [ 840545 ] Number type warning in help.c
Turns out that it was a very simple case.
Now I'm onto bugs 840539 and 840543
Oh, and I've dusted off my Sourceforge user ID of joncruz
Index: help.c
===================================================================
RCS file: /cvsroot/inkscape/inkscape/src/help.c,v
retrieving revision 1.2
diff -u -r1.2 help.c
--- help.c 30 Oct 2003 05:52:06 -0000 1.2
+++ help.c 12 Nov 2003 17:45:34 -0000
@@ -47,7 +47,7 @@
w = gtk_window_new (GTK_WINDOW_TOPLEVEL);
gtk_window_set_title (GTK_WINDOW (w), _("About inkscape"));
- gtk_window_set_default_size (GTK_WINDOW (w), sp_document_width (doc), sp_document_height (doc));
+ gtk_window_set_default_size (GTK_WINDOW (w), (gint)(sp_document_width(doc)), (gint)(sp_document_height(doc)) );
#if 1
gtk_window_set_policy (GTK_WINDOW (w), TRUE, TRUE, FALSE);
#endif
Rob. http://members.rogers.com/rcrosbie
_________________________________________________________________ Protect your PC - get McAfee.com VirusScan Online http://clinic.mcafee.com/clinic/ibuy/campaign.asp?cid=3963
Robert Crosbie wrote:
Ok, so I turned off digest mode. In the mean time, excuse the cut and paste below:
This patch doesn't really fix the problem, it only masks the compiler warning.
If gtk_window_set_default_size is expecting gints, then sp_document_width should return a gint instead of a gdouble and the document structure should store its width and height as gint. I don't have the source accessible to me right now, so I can't see how it is defined.
If a width was defined that exceeded INT_MAX, then when the gdouble was cast to a gint, data would be lost. A user would see a windo much smaller than they exected being displayed.
Ahhh... very good points.
Initially my goal was to achieve identical behavior with base SodiPodi. Changing to use explicit casts gets the default behavior. (and, in fact, the warnings don't show up when using gcc to compile, just g++).
However... your point about changing it to do the 'proper' thing is very good. For code like I just touched there are a few choices.
* Change that kind of code to be 'proper' and worry about performance later if it shows up as an issue. * Change to match the C/SodiPodi behavior and mark the code with some 'todo' marker to check for the proper thing. * Change the code to 'proper' and run it through the regression test system to check performance.
(Of course, this applies more to desktop-snap.c and such code)
And in a similar vein, there is the question of casts themselves. What should we do? At the moment we still want the code to compile with plain C compilers and not require C++ yet. However, when adding casts needed to make things happy for g++, the question of using the proper C++ style casts comes up.
Initially, I just changed this
sp_document_width (doc)
to this
(gint)(sp_document_width(doc))
including the extra parenthesis to make conversion to C++ easier later, as in
static_cast<gint>(sp_document_width(doc))
However... if we are going to be taking the approach of fixing things to be more proper while we are in there, then maybe we should use some temporary #defines to get things happy with C and C++?
as in:
INK_STATIC_CAST(gint, sp_document_width(doc)) or something similar.
Jon A. Cruz wrote:
Robert Crosbie wrote:
If a width was defined that exceeded INT_MAX, then when the gdouble was cast to a gint, data would be lost. A user would see a windo much smaller than they exected being displayed.
Ahhh... very good points.
Here ya go. A new and improved patch:
Index: src/help.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/help.c,v retrieving revision 1.2 diff -u -r1.2 help.c --- src/help.c 30 Oct 2003 05:52:06 -0000 1.2 +++ src/help.c 14 Nov 2003 05:24:11 -0000 @@ -27,6 +27,9 @@ return FALSE; }
+#define WINDOW_MIN 20 +#define WINDOW_MAX INT_MAX + void sp_help_about (void) { @@ -47,7 +50,11 @@
w = gtk_window_new (GTK_WINDOW_TOPLEVEL); gtk_window_set_title (GTK_WINDOW (w), _("About inkscape")); - gtk_window_set_default_size (GTK_WINDOW (w), sp_document_width (doc), sp_document_height (doc)); + + gint width = INK_STATIC_CAST( gint, CLAMP( sp_document_width(doc), WINDOW_MIN, WINDOW_MAX ) ); + gint height = INK_STATIC_CAST( gint, CLAMP( sp_document_height(doc), WINDOW_MIN, WINDOW_MAX ) ); + gtk_window_set_default_size (GTK_WINDOW (w), width, height ); + #if 1 gtk_window_set_policy (GTK_WINDOW (w), TRUE, TRUE, FALSE); #endif
participants (2)
-
Jon A. Cruz
-
Robert Crosbie