[Bug 721764] souphttpsrc: Add ability to do HTTP session logging

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=721764
  GStreamer | gst-plugins-good | git

--- Comment #6 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-01-09 17:20:10 UTC ---
(In reply to comment #5)
> (In reply to comment #4)
> > Review of attachment 265810 [details] [details]:
> > 
> > [..] 
> > ::: ext/soup/gstsouputils.c
> > @@ +31,3 @@
> > +gst_soup_util_init (void)
> > +{
> > +  GST_DEBUG_CATEGORY_INIT (gst_soup_utils, "gstsouputils", 0, "Soup utils");
> > 
> > Or just do that in plugin_init() and make the debug category extern. No need
> > for another function here
> 
> Do I understand correctly that would require the calling code to
> be aware of souputil's debug category name? What are we gaining
> really?. I'm OK with taking it out though.

GST_DEBUG_CATEGORY_EXTERN(gst_soup_utils) in the utils.c
GST_DEBUG_CATEGORY(gst_soup_utils) in plugin.c
GST_DEBUG_CATEGORY_INIT(gst_soup_utils) in plugin.c

That's three lines vs. another function that needs to be called from two places
and also would need some locking for thread safety :)

> > @@ +84,3 @@
> > +
> > +  if (logger) {
> > +    GST_WARNING ("We already have a logger. Not attaching another one");
> > 
> > All these checks should be a g_return_if_fail() IMHO. They are programming
> > errors
> 
> Code then will misbehave if G_DISABLE_CHECKS
> was set for glib?. Now (!level) is being read from
> a writable user facing prop, I'm guessing you didn't
> meant to include that one in your "they"?

I did, setting the GObject property to another value will cause a g_critical()
and is also considered a programming error. Adding a g_return_if_fail() for
that seems ok.

> About the checking for GST_CAT_DEFAULT, that macro is
> expanding to gst_soup_utils on that file already. What's
> wrong with it?

Nothing, I forgot that you defined it to something sensible above :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.