[Webkit-unassigned] [Bug 178160] [GStreamer][MSE] Trim space between codecs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=178160

--- Comment #28 from Alicia Boya García <aboya at igalia.com> ---
Comment on attachment 325016
  --> https://bugs.webkit.org/attachment.cgi?id=325016
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325016&action=review

>> Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:850
>> +            return true;
> 
> You don't need the variable, just if to the fnmatch and return true.
> 
> Btw, the name of the variable should be doesCodecMatchPattern according to https://webkit.org/code-style-guidelines/#names

I know the variable is syntactically redundant, but I introduced it on purpose to make the code easier to understand. 

Many people unfamiliar with `fnmatch()` would assume that `!fnmatch(...)` means *string does not match pattern* but turns out it is actually the opposite.

Introducing the variable I'm splitting the logic in two pieces that are easier to analyze: First it says "this variable tells if the codec matches the pattern, I know that by calling !fnmatch()" (when the reader gets surprised on the negation operator, they can consult `man fnmatch` like I did in the original code to confirm that it's actually intended). Then it says "if the codec matched (no matter how I computed it), return true".

I agree with the rename though.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171031/f083518a/attachment.html>