GNOME Bugzilla – Bug 527525
port to GIO
Last modified: 2008-06-18 18:17:13 UTC
These are the occurences of gnome_vfs calls: $:andre\> grep -R gnome_vfs . ./libgweather/weather-bom.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, bom_finish_read, info); ./libgweather/weather-bom.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, bom_finish_read, info); ./libgweather/weather-bom.c: gnome_vfs_async_open(&info->bom_handle, url, GNOME_VFS_OPEN_READ, ./libgweather/weather-met.c: g_print("%s", gnome_vfs_result_to_string(result)); ./libgweather/weather-met.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, met_finish_read, info); ./libgweather/weather-met.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, met_finish_read, info); ./libgweather/weather-met.c: gnome_vfs_async_open(&info->met_handle, url, GNOME_VFS_OPEN_READ, ./libgweather/weather.c: gnome_vfs_async_close(handle, close_cb, info); ./libgweather/weather.c: gnome_vfs_async_cancel(info->metar_handle); ./libgweather/weather.c: gnome_vfs_async_cancel(info->iwin_handle); ./libgweather/weather.c: gnome_vfs_async_cancel(info->wx_handle); ./libgweather/weather.c: gnome_vfs_async_cancel(info->met_handle); ./libgweather/weather.c: gnome_vfs_async_cancel(info->bom_handle); ./libgweather/weather-metar.c: g_print("%s", gnome_vfs_result_to_string(result)); ./libgweather/weather-metar.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, metar_finish_read, info); ./libgweather/weather-metar.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, metar_finish_read, info); ./libgweather/weather-metar.c: gnome_vfs_async_open(&info->metar_handle, url, GNOME_VFS_OPEN_READ, ./libgweather/weather-iwin.c: g_print("%s", gnome_vfs_result_to_string(result)); ./libgweather/weather-iwin.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, iwin_finish_read, info); ./libgweather/weather-iwin.c: gnome_vfs_async_read(handle, body, DATA_SIZE - 1, iwin_finish_read, info); ./libgweather/weather-iwin.c: gnome_vfs_async_open(&info->iwin_handle, url, GNOME_VFS_OPEN_READ, ./libgweather/weather-wx.c: gnome_vfs_async_read(handle, buffer, DATA_SIZE - 1, wx_finish_read, info); ./libgweather/weather-wx.c: g_print("%s", gnome_vfs_result_to_string(result)); ./libgweather/weather-wx.c: gnome_vfs_async_read(handle, body, DATA_SIZE -1, wx_finish_read, info); ./libgweather/weather-wx.c: gnome_vfs_async_open(&info->wx_handle, url, GNOME_VFS_OPEN_READ,
*** Bug 527453 has been marked as a duplicate of this bug. ***
Created attachment 109600 [details] [review] patch to port libgweather to use libsoup for http Not quite what the summary calls for, but it simplifies the code quite a bit relative to using either gnome-vfs or gio
Thanks Dan! That's much less code, it must be good :-) A few comments, without looking at the rest of the code: + I'm going to assume your usage of libsoup is correct ;-) + in some places, you're mixing tabs and spaces. I can see this in wx_finish() at least. Choose whatever is the most appropriate in the file. + please change the pkg-config file (gweather.pc.in, iirc) to replace the gnome-vfs reference by libsoup + I'm not sure, but it sounds weird to calculate something about the sun in each request_done since this function is called for each download. Shouldn't we only do it if it has something to do with the sun? + you moved "g_free (info->forecast)" from iwin_finish_open() to the init function (sorry, don't have the real name of the function -- maybe you should play with http://marnanel.blogspot.com/2007/05/tips-how-to-get-function-names-in-svn.html ?). I guess this could be argued. If you get a temporary network failure, it makes sense to keep the previous data as a cache. However, if you get a longer network failure, it's going to be stale -- so you're doing right. + in weather-bom.c, info->forecast used to be freed + in weather-met.c, info->forecast used to be freed + you're not translating all the g_warning(). I guess it's useless to translate them. It won't mean anything to the user anyway... + in wx_finish, you put "g_object_ref (info->radar);" I'm wondering: maybe this should be g_object_ref_sink()?
(In reply to comment #3) > + in some places, you're mixing tabs and spaces. I can see this in wx_finish() > at least. Choose whatever is the most appropriate in the file. The file was already inconsistent... I've fixed it now to use tabs everywhere. (I think every file in libgweather uses a different code style... :) > + please change the pkg-config file (gweather.pc.in, iirc) to replace the > gnome-vfs reference by libsoup oops, fixed > + I'm not sure, but it sounds weird to calculate something about the sun in > each request_done since this function is called for each download. Shouldn't we > only do it if it has something to do with the sun? I agree, but that's how it worked before... > + you moved "g_free (info->forecast)" from iwin_finish_open() to the init > function... > I guess this could be argued. If you get a temporary network failure, it > makes sense to keep the previous data as a cache. However, if you get a longer > network failure, it's going to be stale -- so you're doing right. So we're keeping that change? > + in weather-bom.c, info->forecast used to be freed > > + in weather-met.c, info->forecast used to be freed This is part of the previous change; these methods are only called from iwin_start_open(), so since we've moved the free to there, we don't need it here. > + you're not translating all the g_warning(). I guess it's useless to > translate them. It won't mean anything to the user anyway... I didn't change the translation status of any of the warnings; the ones that were translated before are translated now, and the ones that weren't before still aren't. (I just changed them to include the HTTP status code and reason phrase, to provide a little bit more debugging info.) > + in wx_finish, you put "g_object_ref (info->radar);" I'm wondering: maybe > this should be g_object_ref_sink()? Yeah, I suspect that that ref is just wrong, but it was there before, so I kept it, in case something else in gweather-applet or something is unreffing it an extra time.
Created attachment 109617 [details] [review] revised libsoup patch
vuntz: *ping*
Let's just commit. The only comment I might like to see addressed is the one about wx_finish & g_object_ref(). Since we're in a unstable cycle, I guess it's the right time to fix this and we'll have time to fix things that might be broken because of this.
Committed. (The patch I committed is actually a little different, having been updated to match the code cleanup patch) (In reply to comment #7) > Let's just commit. The only comment I might like to see addressed is the one > about wx_finish & g_object_ref(). Since we're in a unstable cycle, I guess it's > the right time to fix this and we'll have time to fix things that might be > broken because of this. Yeah, I expect some more code cleanup patches to come.