[Webkit-unassigned] [Bug 149496] [GTK] Use mobile user-agent on tablet

[bugzilla-daemon at webkit.org]

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

Adrian Perez <aperez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aperez at igalia.com
 Attachment #403517|review+                     |review-
              Flags|                            |

--- Comment #22 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 403517
  --> https://bugs.webkit.org/attachment.cgi?id=403517
Patch

The logic in the patch looks good overall, but there are still a few
things that I would prefer to see ironed out before landing. At any
rate, thanks a ton for working on it!

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

> Source/WebCore/platform/glib/UserAgentGLib.cpp:52
> +    if (!g_file_get_contents("/etc/machine-info", &buffer.outPtr(), nullptr, nullptr))

Please use a GError here and emit a warning if the error is other
than G_IO_ERROR_NOT_FOUND, which will make it much easier to figure
out the cause when things don't work. In general silently skipping
over errors is a bad idea.

> Source/WebCore/platform/glib/UserAgentGLib.cpp:72
> +    if (g_file_get_contents("/sys/class/dmi/id/chassis_type", &buffer.outPtr(), nullptr, nullptr)) {

Use a GError here as well, please.

> Source/WebCore/platform/glib/UserAgentGLib.cpp:73
> +        int t = atoi(buffer.get());

Please use strtol() instead and check its result for conversion errors.
Even better, use WTF::toIntegralType() which also can report errors.

> Source/WebCore/platform/glib/UserAgentGLib.cpp:75
> +        switch (t) {

A comment here indicating which source file in the kernel defines
these constants would be useful, in case somebody need to re-check
it in the future to update the “switch” statement.

> Source/WebCore/platform/glib/UserAgentGLib.cpp:104
> +        int t = atoi(buffer.get());

Ditto: Do not silently skip over all errors, use a checked
numeric conversion function, and add a comment indicating which
Linux kernel source file defines the magic values.

-- 
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/20200706/3e5d63d4/attachment.htm>