From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-11-25 20:55:57
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Summary: ParamSpec value_validate() on non ref counted boxed Classification: Bindings Product: gnome-perl Version: unspecified OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: Normal Component: Glib AssignedTo: gtk...@li... ReportedBy: us...@zi... QAContact: gtk...@li... GNOME target: --- GNOME version: --- Created an attachment (id=175274) View: https://bugzilla.gnome.org/attachment.cgi?id=175274 Review: https://bugzilla.gnome.org/review?bug=635809&attachment=175274 patch I think the code I made for value_validate() is no good on non reference counted boxed objects like GdkRectangle. In the 1.220 gperl_value_from_sv() copies to the local GValue, gperl_sv_from_value() makes a pointer into there, ie. doesn't copy), but then the memory is gone by g_value_unset of that local GValue. In the cvs head it's slightly better. gperl_value_from_sv() makes only a pointer to the incoming SV's memory, not a copy, gperl_sv_from_value() too makes a pointer not a copy, so you end up with an alias of the incoming SV. Which is ok so long as you don't naively discard that original SV. I'm looking at the change below to give back the input SV if value_validate() says it's ok already, and to copy out the GValue if it says it was changed. This would be for both 1.220 and the cvs head. The only thing I'm slightly unsure is whether setting the GValue to point into the original SV is right. Is g_param_value_validate() expected to set in a new block of memory if it makes a change, or will it modify the content in-place? I don't suppose anyone has every actually created a paramspec subtype taking a GdkRectangle or similar and mangled in value_validate(). -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-11-25 20:59:17
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified --- Comment #1 from Kevin Ryde <us...@zi...> 2010-11-25 20:59:06 UTC --- Created an attachment (id=175275) View: https://bugzilla.gnome.org/attachment.cgi?id=175275 Review: https://bugzilla.gnome.org/review?bug=635809&attachment=175275 test case Bit of code for GdkRegion.t exercising value_validate(). Don't think there's any non ref counted boxed in base Glib to try this on. -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-11-25 21:07:28
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified --- Comment #2 from Kevin Ryde <us...@zi...> 2010-11-25 21:07:16 UTC --- Created an attachment (id=175276) --> (https://bugzilla.gnome.org/attachment.cgi?id=175276) failing program This program in 1.220 gives a bad return like newrect values: 0 2 3 4 or in the cvs head newrect values: 166513440 2 3 4 In the head it's ok if you don't "undef $rect" since that discards the memory aliased by $newrect. Dunno if that should be merely some pod instead of some code, but returning the original SV helps avoid that problem (by effectively assigning $newrect=$rect instead of mucking about with new boxed pointers). -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-11-27 13:53:25
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Torsten Schoenfeld <kaffeetisch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kaf...@gm... --- Comment #3 from Torsten Schoenfeld <kaf...@gm...> 2010-11-27 13:53:13 UTC --- (In reply to comment #0) > I'm looking at the change below to give back the input SV if value_validate() > says it's ok already, and to copy out the GValue if it says it was changed. > This would be for both 1.220 and the cvs head. Sounds correct to me. > The only thing I'm slightly unsure is whether setting the GValue to point into > the original SV is right. Is g_param_value_validate() expected to set in a new > block of memory if it makes a change, or will it modify the content in-place? > I don't suppose anyone has every actually created a paramspec subtype taking a > GdkRectangle or similar and mangled in value_validate(). It seems like a value_validate vfunc is allowed to do both, modify in-place and install a modified chunk of memory. param_string_validate (in gparamspecs.c), for example, can apparently do both depending on whether G_VALUE_NOCOPY_CONTENTS is set (i.e. whether g_value_set_static_string was used). In git master, we use g_value_set_static_boxed so that the G_VALUE_NOCOPY_CONTENTS flag will be set. So I think any well-behaved paramspec implementation will then have to refrain from modifying the memory in-place. The stable-1-22 branch is safe because it uses g_value_set_boxed which copies the boxed object. Does that sound correct to you? If so, I'll go ahead and commit your patches. -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-11-28 00:24:54
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Kevin Ryde <user42> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175274|0 |1 is obsolete| | --- Comment #4 from Kevin Ryde <us...@zi...> 2010-11-28 00:24:43 UTC --- Created an attachment (id=175394) View: https://bugzilla.gnome.org/attachment.cgi?id=175394 Review: https://bugzilla.gnome.org/review?bug=635809&attachment=175394 patch Sounds close. I tweaked the comments per attached. Feel free to write something more. The only thing I further wondered was whether a GValue to SV like this might look at the NOCOPY flag, or whatever the public form of that is, and copy if the memory belongs to the GValue or just use the pointer if it belongs to someone else. That might apply generally in a few places, or it might be a bit too subtle, I'm not sure. -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-12-12 14:13:54
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Torsten Schoenfeld <kaffeetisch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED --- Comment #5 from Torsten Schoenfeld <kaf...@gm...> 2010-12-12 14:13:40 UTC --- I don't see a public form of G_VALUE_NOCOPY_CONTENTS, so I think this is the best we can do. Patch and test case committed. Thanks! -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-12-12 14:13:55
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Torsten Schoenfeld <kaffeetisch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175394|none |committed status| | -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |
From: gnome-perl (bugzilla.gnome.o. <bug...@gn...> - 2010-12-12 14:13:55
|
https://bugzilla.gnome.org/show_bug.cgi?id=635809 gnome-perl | Glib | unspecified Torsten Schoenfeld <kaffeetisch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175275|none |committed status| | -- Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug. You are the assignee for the bug. |