[Bug 751371] souphttpsrc: custom query of user-agent and cookie

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=751371

Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #307389|none                        |needs-work
             status|                            |

--- Comment #8 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
Review of attachment 307389:
 --> (https://bugzilla.gnome.org/review?bug=751371&attachment=307389)

::: ext/soup/gstsouphttpsrc.c
@@ +127,3 @@
 };

+#define DEFAULT_USER_AGENT           "Mozilla/5.0 (Linux; Tizen 2.3; SAMSUNG
SM-Z130H) AppleWebKit/537.3 (KHTML, like Gecko) Version/2.3 Mobile
Safari/537.3"

You can of course override this in your app, but we are not going to pretend
that GStreamer is Mozilla/Tizen. This also surpass the 80 character coding
style limit.

@@ +479,3 @@
   }

+  src->cookie_jar = NULL;

Everything is already set to 0 in a GObject structure.

@@ +487,2 @@
 static void
+gst_soup_http_src_update_cookie (GstSoupHTTPSrc * src)

In my opinion, you should improve readability of this function, try to avoid
checking for too many condition (which do affectation, array =) if that same
if. Here's an example:

if (!src->sesion)
  return;

if (!src->cookie_jar) {
 ...
}

if (!src->cookie_jar)
  return;

@@ +525,3 @@
+
+  cookies = NULL;
+  if ((src->cookie_jar) &&

Just deal with no cookie_jar first, this will make things more readable.

@@ +530,3 @@
+    array = cookies;
+    for (c = cookie_list; c; c = c->next) {
+      *array++ = soup_cookie_to_set_cookie_header ((SoupCookie *) (c->data));

This is pretty ugly, I'd prefer if we avoid this kind of constructions.

@@ +740,3 @@
       break;
     case PROP_COOKIES:
+      g_value_set_boxed (value, gst_soup_http_src_get_cookie (src));

This leaks the cookies array. Use g_value_take_boxed(). It looks like this is
ancient bug though.

@@ +1706,3 @@
+
+    SoupURI *uri = NULL;
+    SoupCookie *cookie;

You could declare this inside the scope of the for loop using it.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.