Skip to content

Derby: Convert the audioop module to use Argument Clinic #64332

Closed
Closed
Derby: Convert the audioop module to use Argument Clinic#64332
Assignees
serhiy-storchaka
Labels
extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

Description

@serhiy-storchaka
serhiy-storchaka
opened on Jan 5, 2014
BPO 20133
Nosy @larryhastings, @serhiy-storchaka
Dependencies
  • bpo-20142: Argument Clinic: Py_buffer parameters are not initialized
  • bpo-20161: inspect.signature fails on some functions which use Argument Clinic
    		•
    Files
  • audioop_clinic.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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2014-01-25.10:01:08.342>
created_at = <Date 2014-01-05.21:21:04.133>
labels = ['extension-modules', 'type-feature']
title = 'Derby: Convert the audioop module to use Argument Clinic'
updated_at = <Date 2014-01-26.07:38:18.840>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-01-26.07:38:18.840>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2014-01-25.10:01:08.342>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2014-01-05.21:21:04.133>
creator = 'serhiy.storchaka'
dependencies = ['20142', '20161']
files = ['33530']
hgrepos = []
issue_num = 20133
keywords = ['patch']
message_count = 10.0
messages = ['207401', '207595', '207626', '207645', '208084', '208245', '208252', '209181', '209261', '209285']
nosy_count = 3.0
nosy_names = ['larry', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20133'
versions = ['Python 3.4']

    Activity

    1. serhiy-storchaka commented on Jan 5, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 5, 2014
      MemberAuthor

      Here is a patch which converts the audioop module to use Argument Clinic. 26 functions are converted. Also it adds docstrings for audioop functions.

      There are two problems:

      • Test test_string crashes. Perhaps here is a bug in Argument Clinic (Py_buffer arguments are not initialized).

      • Pydoc for ratecv (and only for this function) returns:

        ratecv =

    2. serhiy-storchaka
      added
      extension-modulesC modules in the Modules dir
      type-featureA feature request or enhancement
      on Jan 5, 2014

    3. larryhastings commented on Jan 7, 2014

      @larryhastings
      larryhastings
      on Jan 7, 2014
      Contributor

      Can you refresh the patch? I think all the problems you cited are fixed, and also the comments Argument Clinic uses were all changed. I'll review when you have a fresh patch.

    4. larryhastings
      changed the title [-]Convert the audioop module to use Argument Clinic[/-] [+]Derby: Convert the audioop module to use Argument Clinic[/+] on Jan 7, 2014

    5. serhiy-storchaka commented on Jan 7, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 7, 2014
      MemberAuthor

      Here is refreshed patch.

    6. larryhastings commented on Jan 8, 2014

      @larryhastings
      larryhastings
      on Jan 8, 2014
      Contributor

      Serhiy: Assigning to you because you wrote a patch; if you don't want the issue, sorry, please undo it.

    7. larryhastings
      assigned
      serhiy-storchaka
      on Jan 8, 2014

    8. serhiy-storchaka commented on Jan 14, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 14, 2014
      MemberAuthor

      Patch updated to tip (a little different generated code for Py_buffer).

      I don't want to use return converters here because I want to add support for float32 and float64 in 3.5.

    9. larryhastings commented on Jan 16, 2014

      @larryhastings
      larryhastings
      on Jan 16, 2014
      Contributor

      I don't understand. Python already has "double" which is float64. Why would Python need a second floating-point type which isn't as good?

      In any case, if somehow you get float32 in, we could just change the return converters then. So I would still prefer float return converters for now.

    10. serhiy-storchaka commented on Jan 16, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 16, 2014
      MemberAuthor

      I mean I want to add support for float32 and float64 in audioop functions. In that case return value of getsample(), max(), ets will be no longer integer for float format.

    11. python-dev commented on Jan 25, 2014

      @python-dev
      python-devmannequin
      on Jan 25, 2014
      Mannequin

      New changeset d4099b8a7d0f by Serhiy Storchaka in branch 'default':
      Issue bpo-20133: The audioop module now uses Argument Clinic.
      http://hg.python.org/cpython/rev/d4099b8a7d0f

    12. serhiy-storchaka
      closed this as completedon Jan 25, 2014

    13. larryhastings commented on Jan 26, 2014

      @larryhastings
      larryhastings
      on Jan 26, 2014
      Contributor

      Christian Heimes just posted this to python-dev:

      Coverity has detected an issue in this commit:

      ** CID 1164423: Division or modulo by zero (DIVIDE_BY_ZERO)
      /Modules/audioop.c: 1375 in audioop_ratecv_impl()

      ________________________________________________________________________________________________________
*** CID 1164423:  Division or modulo by zero  (DIVIDE_BY_ZERO)
/Modules/audioop.c: 1375 in audioop_ratecv_impl()
1369                without spurious overflow is the challenge; we can
1370                settle for a reasonable upper bound, though, in this
1371                case ceiling(len/inrate) * outrate. */
1372
1373             /* compute ceiling(len/inrate) without overflow */
1374             Py_ssize_t q = len > 0 ? 1 + (len - 1) / inrate : 0;
>>>     CID 1164423:  Division or modulo by zero  (DIVIDE_BY_ZERO)
>>>     In expression "9223372036854775807L / q", division by expression
"q" which may be zero has undefined behavior.
1375             if (outrate > PY_SSIZE_T_MAX / q / bytes_per_frame)
1376                 str = NULL;
1377             else
1378                 str = PyBytes_FromStringAndSize(NULL,
1379                                                 q * outrate *
bytes_per_frame);
1380         }

    14. serhiy-storchaka commented on Jan 26, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 26, 2014
      MemberAuthor

      Christian Heimes just posted this to python-dev:

      See bpo-20394. This is unrelated to clicalization patch.

    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions