Created on 2020-07-19 10:36 by fj92f3jj923f923, last changed 2020-07-27 03:29 by methane. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 21544 | closed | fj92f3jj923f923, 2020-07-19 10:36 | |
| PR 21634 | merged | fj92f3jj923f923, 2020-07-27 01:46 | |
| Messages (11) | |||
|---|---|---|---|
| msg373955 - (view) | Author: (fj92f3jj923f923) * | Date: 2020-07-19 10:36 | |
Hi, all! strdup implementation inside cpython/Python/strdup.c is not the best one. It calls strlen + strcpy, which is the same as calling strlen twice + memcpy. So I replaced it by the call of strlen + memcpy. It is easy to look any implementation in any library. Here for example: https://code.woboq.org/userspace/glibc/string/strdup.c.html So I fixed it here: https://github.com/python/cpython/pull/21544 |
|||
| msg373976 - (view) | Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2020-07-19 23:54 | |
I don't think we need to spend much effort on this implementation, since it will only be used if the system libc doesn't have a strdup already. |
|||
| msg373986 - (view) | Author: (fj92f3jj923f923) * | Date: 2020-07-20 06:23 | |
Got it. But the fix is quite easy as I see, and does not require much affort too. |
|||
| msg373989 - (view) | Author: Inada Naoki (methane) * |
Date: 2020-07-20 06:58 | |
Can we just remove strdup.c? How about hypot.c? |
|||
| msg374199 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2020-07-24 18:26 | |
Either remove it or close this issue as won’t fix. |
|||
| msg374241 - (view) | Author: hai shi (shihai1991) *  |
Date: 2020-07-25 05:00 | |
What's change for the performance benchmark? |
|||
| msg374302 - (view) | Author: Inada Naoki (methane) * |
Date: 2020-07-26 05:34 | |
@fj92f3jj923f923 Would you update the PR to remove the strdup.c? |
|||
| msg374319 - (view) | Author: Guido van Rossum (gvanrossum) * |
Date: 2020-07-26 15:11 | |
There’s probably also something in configure.in that can switch this on. |
|||
| msg374345 - (view) | Author: (fj92f3jj923f923) * | Date: 2020-07-26 21:23 | |
I can give a try with removing Please, wait Thanks everyone for giving a chance to make a PR :) |
|||
| msg374353 - (view) | Author: (fj92f3jj923f923) * | Date: 2020-07-27 02:00 | |
Created a new PR for removal of strdup (Hope it is correct) https://github.com/python/cpython/pull/21634 Closed old one |
|||
| msg374359 - (view) | Author: Inada Naoki (methane) * |
Date: 2020-07-27 03:28 | |
New changeset 5798f787779006a94a55ec74a86da4627de90146 by wasiher in branch 'master': bpo-41340: Removed fallback implementation for strdup (GH-21634) https://github.com/python/cpython/commit/5798f787779006a94a55ec74a86da4627de90146 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2020-07-27 03:29:02 | methane | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
| 2020-07-27 03:28:52 | methane | set | messages: + msg374359 |
| 2020-07-27 02:00:03 | fj92f3jj923f923 | set | messages: + msg374353 |
| 2020-07-27 01:46:10 | fj92f3jj923f923 | set | pull_requests: + pull_request20776 |
| 2020-07-26 21:23:49 | fj92f3jj923f923 | set | messages: + msg374345 |
| 2020-07-26 15:22:00 | gvanrossum | set | nosy:
- gvanrossum |
| 2020-07-26 15:11:17 | gvanrossum | set | messages: + msg374319 |
| 2020-07-26 05:34:22 | methane | set | messages: + msg374302 |
| 2020-07-25 05:00:43 | shihai1991 | set | nosy:
+ shihai1991 messages: + msg374241 |
| 2020-07-24 18:26:41 | gvanrossum | set | nosy:
+ gvanrossum messages: + msg374199 |
| 2020-07-20 06:58:57 | methane | set | nosy:
+ methane messages: + msg373989 |
| 2020-07-20 06:23:00 | fj92f3jj923f923 | set | messages: + msg373986 |
| 2020-07-19 23:54:49 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages: + msg373976 |
| 2020-07-19 10:36:53 | fj92f3jj923f923 | set | keywords:
+ patch stage: patch review pull_requests: + pull_request20686 |
| 2020-07-19 10:36:04 | fj92f3jj923f923 | create | |