Bug 36217 - Wrong env variable handling in _imp_setProcessLocale
Summary: Wrong env variable handling in _imp_setProcessLocale
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-13 11:08 UTC by Christian Dywan
Modified: 2011-04-15 04:00 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
proof-of-concept (3.72 KB, patch)
2011-04-13 11:12 UTC, Christian Dywan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Dywan 2011-04-13 11:08:58 UTC
So I noticed, _imp_setProcessLocale, defined separately in ure/sal/osl/unx and ure/sal/osl/os2 has an obvious logic bug and a theoretical flaw:

if LANG is set, LC_CTYPE is set on systems with a 3-argument setenv, LANG otherwise.

The theoretical aspect is that the given systems are different in the two versions of the function, and it strikes me that this really should have one setenv wrapper shared amongst all files.

I don't know where to put such shared function, however, I'll attach a proof-of-concept patch where the function is only defined in ure/sal/osl/unxnlsupport.c.
Comment 1 Christian Dywan 2011-04-13 11:12:46 UTC
Created attachment 45584 [details]
proof-of-concept
Comment 2 Jan Holesovsky 2011-04-14 16:24:48 UTC
Christian: Thank you very much for your patch!  Can you please confirm that it is LGPL3 / MPL licensed?

You are right, the LC_CTYPE setting smells a copy'n'paste ;-)  The thing is that this seems not to be compiled at all on LINUX || SOLARIS || NETBSD || FREEBSD || OPENBSD || DRAGONFLY - ure/sal/osl/os2 is OS/2-related only (I suspect a platform we do not support any more), and the _imp_setProcessLocale() in unx/nlsupport.c is actually compiled only in !(LINUX || SOLARIS || NETBSD || FREEBSD || OPENBSD || DRAGONFLY) case - so only the #if defined(AIX) has any value in the original code.

Either way - I fixed it to build, minimized it, and pushed as:

http://cgit.freedesktop.org/libreoffice/ure/commit/?h=libreoffice-3-4&id=e75d09e4e89d3eda7ddaa2c085678edf9a21293f

:-)  Please send your further patches directly to the libreoffice@lists.freedesktop.org, ideally as the result of git format-patch, so that we have even your name / email address right.  More info here:

http://wiki.documentfoundation.org/Development/Patch_Handling_Guideline
Comment 3 Christian Dywan 2011-04-15 04:00:00 UTC
Thanks! Yes LGPL3/ MPL is fine. I will use the list in the future. I was only a diff since it wasn't a final patch, I should've made that clearer.