Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyOS_mystricmp unused and no longer available #62803

Closed
tiran opened this issue Jul 30, 2013 · 11 comments
Closed

PyOS_mystricmp unused and no longer available #62803

tiran opened this issue Jul 30, 2013 · 11 comments
Labels
build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@tiran
Copy link
Member

tiran commented Jul 30, 2013

BPO 18603
Nosy @loewis, @birkenfeld, @vstinner, @larryhastings, @tiran, @benjaminp, @jwilk
Files
  • 18603.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-10-22.08:39:03.998>
    created_at = <Date 2013-07-30.23:58:45.205>
    labels = ['interpreter-core', 'build']
    title = 'PyOS_mystricmp unused and no longer available'
    updated_at = <Date 2013-12-21.16:28:27.716>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2013-12-21.16:28:27.716>
    actor = 'jwilk'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-10-22.08:39:03.998>
    closer = 'christian.heimes'
    components = ['Interpreter Core']
    creation = <Date 2013-07-30.23:58:45.205>
    creator = 'christian.heimes'
    dependencies = []
    files = ['31357']
    hgrepos = []
    issue_num = 18603
    keywords = ['patch', '3.3regression']
    message_count = 11.0
    messages = ['193965', '195564', '200759', '200793', '200795', '200796', '200884', '200888', '200891', '200895', '200958']
    nosy_count = 9.0
    nosy_names = ['loewis', 'georg.brandl', 'vstinner', 'larry', 'christian.heimes', 'benjamin.peterson', 'jwilk', 'Arfrever', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue18603'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @tiran
    Copy link
    Member Author

    tiran commented Jul 30, 2013

    The two functions PyOS_strnicmp() and PyOS_stricmp() from Python/pystrcmp.c are no longer used by any function in the core. Because no module references any object from Python/pystrcmp.c the object file is not included in the Python binary:

    $ nm -P python | grep ^PyOS | sort
    PyOS_AfterFork T 000000000044040e 0000000000000036
    PyOS_double_to_string T 00000000005da3de 0000000000000119
    PyOS_FiniInterrupts T 0000000000440380 000000000000000b
    PyOS_getsig T 0000000000423994 0000000000000041
    PyOS_InitInterrupts T 00000000004402f5 000000000000008b
    PyOS_InputHook B 000000000095d478 0000000000000008
    PyOS_InterruptOccurred T 000000000044038b 000000000000003e
    PyOS_ReadlineFunctionPointer B 000000000095d5c8 0000000000000008
    PyOS_Readline T 000000000063ffb5 000000000000012e
    PyOS_setsig T 00000000004239d5 0000000000000078
    PyOS_snprintf T 000000000041c7bc 00000000000000b8
    PyOS_StdioReadline T 000000000063fe1c 0000000000000199
    PyOS_string_to_double T 00000000005d9b50 0000000000000146
    PyOS_strtol T 00000000005d3ded 00000000000000d6
    PyOS_strtoul T 00000000005d3a00 00000000000003ed
    PyOS_vsnprintf T 000000000041c874 00000000000000c4

    Neither Python 2.7 nor 3.3+ include the functions in their binaries. 2.6 and 3.2 are not affected. The functions are part of the documented and stable API but apparently they are not used very often.

    @tiran tiran added the build The build process and cross-build label Jul 30, 2013
    @tiran
    Copy link
    Member Author

    tiran commented Aug 18, 2013

    Here is a simple patch based on the approach in Objects/object.c

    @tiran tiran added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 18, 2013
    @tiran
    Copy link
    Member Author

    tiran commented Oct 21, 2013

    ping RMs

    @benjaminp
    Copy link
    Contributor

    Uggg, can we just kill them if we've already released versions without them?

    @tiran
    Copy link
    Member Author

    tiran commented Oct 21, 2013

    Fine with me, however ...

    Technically all our source releases still contain the code. AFAIK all shared library builds, too. Just standard builds of Python are lacking the object file.

    @vstinner
    Copy link
    Member

    The function doesn't hurt, please don't touch stable versions: only remove it from 3.4. I don't understand the tag "keywords: 3.3regression". Is this issue a bug, or even a compile error??

    @birkenfeld
    Copy link
    Member

    Well, if the functions are part of the stable ABI (which it seems they are, as they don't fall under some exclusion defined by PEP-384) they shouldn't be removed if we take PEP-384 seriously.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 22, 2013

    The patch looks good to me. The stable ABI was originally intended for Windows only, where the functions are guaranteed to be included in python3.dll (as the .def file references them); they are included in python3X.dll as link.exe doesn't do the same object file omission that /bin/ld does. But I agree in principle that the proposed solution is better (and it is the only approach that works with /bin/ld).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 22, 2013

    New changeset 4c4f31a1b706 by Christian Heimes in branch '3.3':
    Issue bpo-18603: Ensure that PyOS_mystricmp and PyOS_mystrnicmp are in the
    http://hg.python.org/cpython/rev/4c4f31a1b706

    New changeset b5cc822d5bf0 by Christian Heimes in branch 'default':
    Issue bpo-18603: Ensure that PyOS_mystricmp and PyOS_mystrnicmp are in the
    http://hg.python.org/cpython/rev/b5cc822d5bf0

    New changeset dc9f17f10899 by Christian Heimes in branch '2.7':
    Issue bpo-18603: Ensure that PyOS_mystricmp and PyOS_mystrnicmp are in the
    http://hg.python.org/cpython/rev/dc9f17f10899

    @tiran
    Copy link
    Member Author

    tiran commented Oct 22, 2013

    Thanks for your input. I have applied the patch to 2.7, 3.3 and 3.4. A couple of 3rd party were bitten by the missing symbols. A Google search reveals a couple of them https://www.google.de/search?q=PyOS_mystricmp+missing

    @tiran tiran closed this as completed Oct 22, 2013
    @benjaminp
    Copy link
    Contributor

    Someone would have to be trying fairly hard to use those functions on Windows because this is in the header:

    #ifdef MS_WINDOWS
    #define PyOS_strnicmp strnicmp
    #define PyOS_stricmp stricmp
    #else
    #define PyOS_strnicmp PyOS_mystrnicmp
    #define PyOS_stricmp PyOS_mystricmp
    #endif

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants