Issue 26130: redundant local copy of a char pointer in classify in Parser\parser.c (original) (raw)

Issue26130

Created on 2016-01-15 20:36 by Oren Milman, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
patchDiff.txt Oren Milman,2016-01-15 20:36 patch diff review
Messages (6)
msg258326 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-01-15 20:36
In Parser\parser.c in classify, the 'str' parameter is assigned into the local variable 's'. However, 'str' is not used anywhere else in the function, which makes 's' redundant. My proposal is to simply remove 's', and just use 'str' instead. The diff is attached. I played a little with the interpreter, and everything worked as usual. In addition, I run 'python -m test' (on my 64-bit Windows 10) before and after applying the patch, and got the same output: 354 tests OK. 1 test altered the execution environment: test_subprocess 45 tests skipped: test_bz2 test_crypt test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll test_fcntl test_fork1 test_gdb test_grp test_idle test_ioctl test_kqueue test_lzma test_nis test_openpty test_ossaudiodev test_pipes test_poll test_posix test_pty test_pwd test_readline test_resource test_smtpnet test_socketserver test_spwd test_sqlite test_ssl test_syslog test_tcl test_threadsignals test_timeout test_tix test_tk test_ttk_guionly test_ttk_textonly test_urllib2net test_urllibnet test_wait3 test_wait4 test_winsound test_xmlrpc_net test_zipfile64
msg258327 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2016-01-15 21:01
Looks good to me. s was probably left over after a rewrite of the function.
msg258346 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-16 01:37
Yes it looks like this is code left over from removing the “register” keyword in revision 0530aadff696, Issue 18722. I quickly looked over the changes in that revision. I couldn’t find any more clear redundancies. I guess this patch is also applicable to 3.5. Maybe the following comments and copies are not needed either. But these cases look like heavily optimized code, so I would be careful with them: https://hg.python.org/cpython/file/4b1bca0b560f/Objects/stringlib/codecs.h#l40 https://hg.python.org/cpython/file/4b1bca0b560f/Objects/stringlib/codecs.h#l519 https://hg.python.org/cpython/file/4b1bca0b560f/Objects/stringlib/find_max_char.h#l26 https://hg.python.org/cpython/file/4b1bca0b560f/Objects/unicodeobject.c#l4794 https://hg.python.org/cpython/file/4b1bca0b560f/Objects/unicodeobject.c#l4819 The last two have mangled comments /* Help allocation */, which previously read /* Help register allocation */.
msg258348 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-16 01:51
Sorry, the revision removing “register” is e7f6cef7a4cc. Not sure what happened there.
msg262540 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-27 21:43
New changeset a90f5e2b7160 by Berker Peksag in branch 'default': Issue #26130: Remove redundant variable 's' from Parser/parser.c https://hg.python.org/cpython/rev/a90f5e2b7160
msg262541 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-03-27 21:44
Thanks for the patch, Oren!
History
Date User Action Args
2022-04-11 14:58:26 admin set github: 70318
2016-03-27 21:44:15 berker.peksag set status: open -> closedversions: - Python 3.5nosy: + berker.peksagmessages: + resolution: fixedstage: patch review -> resolved
2016-03-27 21:43:34 python-dev set nosy: + python-devmessages: +
2016-01-16 01:51:24 martin.panter set messages: +
2016-01-16 01:37:37 martin.panter set versions: + Python 3.5nosy: + martin.pantermessages: + stage: patch review
2016-01-15 21:01:41 georg.brandl set messages: +
2016-01-15 20:47:24 SilentGhost set nosy: + brett.cannon, georg.brandl, ncoghlan, benjamin.peterson, yselivanov
2016-01-15 20:36:48 Oren Milman create