[Bug 765600] gst_memory_map: delegate logging traces to subclass on failure

[GStreamer (GNOME Bugzilla)]

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

--- Comment #20 from Julien Isorce <julien.isorce at gmail.com> ---
(In reply to Tim-Philipp Müller from comment #18)
> Comment on attachment 337176 [details] [review]
> gstfdmemory: log with GST_INFO instead of GST_ERROR on permission denied
> 
> >--- a/gst-libs/gst/allocators/gstfdmemory.c
> >+++ b/gst-libs/gst/allocators/gstfdmemory.c
> >@@ -110,9 +110,20 @@ gst_fd_mem_map (GstMemory * gmem, gsize maxsize, GstMapFlags flags)
> > 
> >     mem->data = mmap (0, gmem->maxsize, prot, flags, mem->fd, 0);
> >     if (mem->data == MAP_FAILED) {
> >+      GString *msg = g_string_new (NULL);
> >       mem->data = NULL;
> >-      GST_ERROR ("%p: fd %d: mmap failed: %s", mem, mem->fd,
> >+
> >+      g_string_printf (msg, "%p: fd %d: mmap failed: %s", mem, mem->fd,
> >           g_strerror (errno));
> >+      switch (errno) {
> >+        case EACCES:
> >+          GST_INFO ("%s", msg->str);
> >+          break;
> >+        default:
> >+          GST_ERROR ("%s", msg->str);
> >+      }
> >+
> >+      g_string_free (msg, TRUE);
> 
> We're now allocating, printf-ing and freeing a GString (and various helper
> allocs used inside g_string_printf) even if debug logging is completely
> disabled or not active, this seems suboptimal to me :)
> 

Oh right! Thx

> Is there any reason not to just duplicate the statement for GST_INFO and
> GST_ERROR ?

I was thinking that there could be more cases in the future, so bigger switch.

> 
> If you *really* want to avoid this you can also use GST_CAT_LEVEL_LOG() and
> specify the level there.

Thx for the suggestion I will try it to see how it looks.

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