GNOME Bugzilla – Bug 120651
gnome_print_job_new() does not just call g_object_new()
Last modified: 2004-12-22 21:47:04 UTC
_new functions should just call g_object_new() with appropriate property values, but gnome_print_job() new does more, making life difficult for language bindings. Here is a non-API/ABI breaking patch to fix this, by adding a _construct() method, but this should really be a property instead.
Created attachment 19481 [details] [review] job_construct.patch
Please provide patches with "-u 5", otherwise it is extremelly hard to read.
Oh, sorry. Can you manage with this one for now though? I have a lousy connection so I won't be able to redo it for a while.
Pretty please?
Created attachment 20346 [details] [review] job_constuct2.patch
Here (job_construct2.patch) is regular -up version of the patch.
I agree its a problem, but not with the proposed solution. Lets make it a property and init to the default.
Created attachment 20699 [details] [review] job_properties.patch
job_properties.patch, as suggested.
May I apply this?
I am committing this patch to cvs HEAD. I'll test it on gnome-2-4 tomorrow afternoon and submit it then to if appropriate.
gnome_print_job_get_property in this patch is nonsense. Unfortunately I didn't realize that until after I committed it and chased some strange bug through gnumeric and libgnomeprint. (I guess we imagine that we see what we like to see, rather than what the code is really doing.)
committed the fixed patch to gnome-2-4
I don't know what the problem was with gnome_print_job_get_property() and I don't see any fix in cvs. Please explain in future. But, in your change to gnome_print_job_set_property() here, if you are going to ref the config, then I think you need to ref it when using the default too. Otherwise I think you will unref something that you have not reffed: http://cvs.gnome.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvs/gnome&subdir=libgnomeprint%2Flibgnomeprint&command=DIFF_FRAMESET&root=/cvs/gnome&file=gnome-print-job.c&rev1=1.61&rev2=1.62 Or I could be competely wrong - feel free to close this again.
Oh, I see my previous unref problem now, and I see that the new problem is from me too, but I still think it could be a problem.
Hi Murray, sorry about the confusion: The problem was with your gnome_print_job_set_property. If you look at your patch you will see that at the time of the if(config) the config had to be 0, so we never really installed the config but always used the default. Regarding the ref in the fixed gnome_print_job_set_property, in the default case we are swallowing the ref from gnome_print_config_default (since gnome_print_config_default does not have a chanve to release it.) but we don't swallow it for config since it is likely that the caller still needs it. IT is the callers responsibility to release the ref if it does not need the ref anymore. in many (most?) cases the caller will want to hold on to the config.)