Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(88227)

#7442: _localemodule.c: str2uni() with different LC_NUMERIC and LC_CTYPE

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by stefan
Modified:
5 years, 6 months ago
Reviewers:
victor.stinner
CC:
loewis, mark.dickinson, haypo, eric.smith, mcepl, skrah
Visibility:
Public.

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/test/test_locale.py View 1 chunk +17 lines, -0 lines 1 comment Download
Modules/_localemodule.c View 6 chunks +41 lines, -3 lines 5 comments Download

Messages

Total messages: 1
victor.stinner_gmail.com
5 years, 6 months ago #1
http://bugs.python.org/review/7442/diff/1473/Lib/test/test_locale.py
File Lib/test/test_locale.py (right):

http://bugs.python.org/review/7442/diff/1473/Lib/test/test_locale.py#newcode374
Lib/test/test_locale.py:374: locale.setlocale(locale.LC_NUMERIC, 'fi_FI')
You should also try the ps_AF locale, a locale having a non-ASCII decimal point:
http://bugs.python.org/issue13706#msg154099

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c
File Modules/_localemodule.c (right):

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c#newcode172
Modules/_localemodule.c:172: if ((locale = setlocale(category, NULL)) == NULL)
I don't like two instruction on the same line.

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c#newcode174
Modules/_localemodule.c:174: return strdup(locale);
You should use _PyMem_Strdup() to reuse Python memory allocator.

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c#newcode261
Modules/_localemodule.c:261: free(lc_ctype);
replace free() with PyMem_Free(), and so you can remove the check for NULL
(PyMem_Free() guarantee to do nothing if the pointer is NULL).

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c#newcode271
Modules/_localemodule.c:271: goto out;
I prefer reading code from top to bottom. The "out" block must be called "exit",
and should be before failed. So use "goto exit" on success (to skip the error
block).

http://bugs.python.org/review/7442/diff/1473/Modules/_localemodule.c#newcode590
Modules/_localemodule.c:590: "Set the C library's textdomain to domain,
returning the new domain.");
You should fix this typo in a separator commit.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+