[Patches] [ python-Patches-652930 ] math.log(x [,base])

[noreply@sourceforge.net]

Patches item #652930, was opened at 2002-12-12 16:26
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=652930&group_id=5470

Category: Modules
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Raymond Hettinger (rhettinger)
Assigned to: Nobody/Anonymous (nobody)
Summary: math.log(x [,base])

Initial Comment:
Added an optional base argument to math.log().

Inspired by a request to help@python.org.


----------------------------------------------------------------------

>Comment By: Raymond Hettinger (rhettinger)
Date: 2002-12-14 13:18

Message:
Logged In: YES 
user_id=80475

This should do it.  I built a cumulative ref list and checked-
off each one.  Also, removed the strcpy() as requested.



----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-14 13:00

Message:
Logged In: YES 
user_id=21627

When studying C-Python code, please always make mental 
notes for each line of the source asking what objects are 
allocated at this point, and which variables are initialized. 
Then, on an error exit, arrange to deallocate them.

In this version, you are missing the case that num 
computation succeeds but the subsequent newargs 
allocation fails.

Getting it right is really difficult; an error-proof is in the use of 
goto: initialize all PyObject * to NULL, including the result, 
add a label exit at the end of the function,  and put 
Py_XDECREFs there. Of course, you now have to be careful 
with intermediate Py_DECREFs; you'ld need to reset the 
variable to NULL. Then only early return is allowed under this 
strategy before the first object is allocated.

Another issue: I think you can drop the strcpy altogether, by 
passing a static literal.


----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2002-12-14 12:34

Message:
Logged In: YES 
user_id=80475

* Deallocated num object on den failure
* Added bracketed text to \versionchanged

I appreciate the quick and thorough reviews.

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-14 02:37

Message:
Logged In: YES 
user_id=21627

I'm sorry for be that picky, but ... This version loses the
num object into garbage if computation of the den object fails.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2002-12-14 00:42

Message:
Logged In: YES 
user_id=80475

New patch attached.
* removed re-use of newargs tuple
* used PyTuple_SET_ITEM
* trapped potential error exits


----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-13 10:36

Message:
Logged In: YES 
user_id=21627

This still missing a number of potential error exits: the 
Tuple_New can fail (as could have the BuildValue), and the 
loghelpers can fail.

On the second SetItem, please add a comment that mutating 
the tuple is fine.

Potentially, the Divide can also fail, but that probably won't 
matter.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2002-12-13 10:24

Message:
Logged In: YES 
user_id=80475

Revised patch attached.

* Now, only one call to ParseTuple
* Used PyTuple_New instead of Py_BuildValue
* Added \versionchanged{} mark-up

----------------------------------------------------------------------

Comment By: Martin v. Löwis (loewis)
Date: 2002-12-13 05:55

Message:
Logged In: YES 
user_id=21627

I think you should avoid the triple calls to ParseTuple, and 
the intermediate BuildValue call. AFAICT, the ParseTuple in 
loghelper can *only* fail if there is not exactly one argument 
(since it won't care about its type); atleast for log, you made 
it impossible that the argument count is incorrect. For log10, 
making the function a METH_O can do that test.

Building a tuple from an object with BuildValue also seems 
overkill, using PyTuple_New is just as simple.

Please use the \versionchanged (or \versionadded) markup to 
document that the parameter is new to 2.3.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=652930&group_id=5470