Bug 33558 - mesa uses of atof to parse glxserverversion harmfull for any of the non english sessions
Summary: mesa uses of atof to parse glxserverversion harmfull for any of the non engli...
Status: RESOLVED MOVED
Alias: None
Product: Mesa
Classification: Unclassified
Component: GLX (show other bugs)
Version: git
Hardware: Other All
: medium major
Assignee: Ian Romanick
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-26 14:48 UTC by Alban Browaeys
Modified: 2019-09-18 17:43 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
workaround atof behing feed non localized data thus preventing glx 1.4 codepath use (1.44 KB, patch)
2011-01-26 14:48 UTC, Alban Browaeys
Details | Splinter Review
Avoid atof to parse non localized glx server version (9.44 KB, patch)
2011-02-06 16:05 UTC, Alban Browaeys
Details | Splinter Review
avoid atof with a wrapper to handle non localized input (9.55 KB, patch)
2011-02-10 18:32 UTC, Alban Browaeys
Details | Splinter Review

Description Alban Browaeys 2011-01-26 14:48:56 UTC
Created attachment 42563 [details] [review]
workaround atof behing feed non localized data thus preventing glx 1.4 codepath use

In src/glx/glxext.c at least for french users if (atof(priv->serverGLXversion) >= 1.3) { always leads to false as priv->serverGLXVersion is "1.4" etc and in locales that uses comma as the decimal separator instead of dot as for english speakers "1.4" becomes "1" via atof instead of 1.4 the double.

I made a small hack using setlocale to set LC_ALL to C before atof call and restore it afterwards. We may not want to bail out if the saved_locale is null (out of memory).

I have been told other places uses atof in mesa, the others places that does so with the same string (serverGLXVersion) is:
src/gallium/state_trackers/egl/x11/glxinit.c

I do not know if the other are feed with data localized or need to be protected like the previous ones.
Comment 1 Dave Airlie 2011-01-26 14:59:30 UTC
I'm guessing we should port the strtof wrapper from mesa imports.h and use that.
Comment 2 Alban Browaeys 2011-02-06 16:05:03 UTC
Created attachment 43013 [details] [review]
Avoid atof to parse non localized glx server version

The location and the name are far stretched (I do not know where to lay down this function so I added it to glx and made it available through GL/glimports.h . Looks weird though I add no clue).
So here is a patch.
Comment 3 Ian Romanick 2011-02-07 14:01:57 UTC
This part of the patch confuses me:

@@ -526,21 +529,26 @@ getFBConfigs(__GLXscreenConfigs *psc, __GLXdisplayPrivate *priv, int screen)
    LockDisplay(dpy);
 
    psc->configs = NULL;
-   if (atof(priv->serverGLXversion) >= 1.3) {
-      GetReq(GLXGetFBConfigs, fb_req);
-      fb_req->reqType = priv->majorOpcode;

[snip]

+   number = (GLfloat) _GLstrtof(priv->serverGLXversion, &end);
+   if (end && end > priv->serverGLXversion) {
+      if (atof(priv->serverGLXversion) >= 1.3) {
+	 GetReq(GLXGetFBConfigs, fb_req);
+	 fb_req->reqType = priv->majorOpcode;

Why is it necessary to call _GLstrtof and atof?  The documentation for strtof, on which _GLstrtof is based, says:

       If no conversion is performed, zero is returned and the value  of  nptr
       is stored in the location referenced by endptr.

This is the same assumption that the old atof code makes.  We either get the GLX version or, in the case of a malformed string, 0.0.  In any case where the value is less than 1.3, we use the GLX 1.2 protocol.

Unless I've missed something (possible), it should be safe to replace the existing call to atof with a call to the new _GLstrtof.

Also, I think it's better to call the new function __glXStrtof.  This matches the naming convention of other internal GLX functions.
Comment 4 Alban Browaeys 2011-02-10 18:32:38 UTC
Created attachment 43222 [details] [review]
avoid atof with a wrapper to handle non localized input

thank you for the review . I changed the name and fixed the garbage I made. I intended to use "number" not the previous atof function.
Comment 5 GitLab Migration User 2019-09-18 17:43:38 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/74.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.