[Patches] [ python-Patches-505846 ] pyport.h, Wince and errno getter/setter
noreply@sourceforge.net
noreply@sourceforge.net
Fri, 19 Apr 2002 07:47:42 -0700
Patches item #505846, was opened at 2002-01-19 21:13
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=505846&group_id=5470
Category: Core (C code)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Brad Clements (bkc)
Assigned to: Nobody/Anonymous (nobody)
Summary: pyport.h, Wince and errno getter/setter
Initial Comment:
Most of the remaining Windows CE diffs are due to the
lack of errno on Windows CE. There are other OS's that
do not have errno (but they have a workalike method).
At first I had simply commented out all references in
the code to errno, but that quickly became unworkable.
Wince and NetWare use a function to set the per-
thread "errno" value. Although errno #defines (like
ERANGE) are not defined for Wince, they are defined
for NetWare. Removing references to errno would
artificially hobble the NetWare port.
These platforms also use a function to retrieve the
current errno value.
The attached diff for pyport.h attempts to standardize
the access method for errno (or it's work-alike) by
providing SetErrno(), ClearErrno() and GetErrno()
macros.
ClearErrno() is SetErrno(0)
I've found and changed all direct references to errno
to use these macros. This patch must be submitted
before the patches for other modules.
--
I see two negatives with this approach:
1. It will be a pain to think GetErrno() instead
of "errno" when modifying/writing new code.
2. Extension modules will need access to pyport.h for
the target platform.
In the worst case, directly referencing errno instead
of using the macros will break only those platforms
for which the macros do something different. That is,
Wince and NetWare.
--
An alternative spelling/capitalization of these macros
might make them more appealing. Feel free to make a
suggestion.
--
It's probably incorrect for me to use SetErrno() as a
function, such as
SetErrno(1);
I think the semi-colon is not needed, but wasn't
entirely certain. On better advice, I will fix these
statements in the remaining source files if this patch
is accepted.
----------------------------------------------------------------------
>Comment By: Jack Jansen (jackjansen)
Date: 2002-04-19 16:47
Message:
Logged In: YES
user_id=45365
Brad,
I think this patch might be asking for too much. You're asking that all accesses to errno be replaced by GetErrno() or SetErrno() calls, really...
And for many cases there is a workaround, where you don't have to change user code (i.e. the normal C code still uses what it thinks is an errno variable). On my system errno is
#define errno (*__error())
and the __error() routine returns a pointer to the errno-variable for the current thread. For the GetErrno function this would be good enough, and with a bit of effort you could probably get it to work for the Set function too (possibly by doing the actual Set work in the next Get call).
----------------------------------------------------------------------
Comment By: Brad Clements (bkc)
Date: 2002-02-12 00:17
Message:
Logged In: YES
user_id=4631
Hi folks,
I need to proceed with the port to NetWare so I have
something to demo at Brainshare in March. Unfortunately
future patches from me will include both WINCE and NetWare
specific patches, though hopefully there won't be much
other than config.h and this patch (which is required for
NetWare).
Is there anything I can do to make this patch more
acceptable? Send a bottle of wine, perhaps? ;-)
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-01-29 00:39
Message:
Logged In: YES
user_id=33168
Tim, I can check in or do whatever else needs to be done
to check this in and move this forward. How do you want to
procede?
Brad, I think most people are pretty busy right now.
----------------------------------------------------------------------
Comment By: Brad Clements (bkc)
Date: 2002-01-29 00:19
Message:
Logged In: YES
user_id=4631
Hi folks, just wondering if this patch is going to be
rejected, or if you're all too busy and I have to be more
patient ;-)
I have a passle of Python-CE folks waiting on me to finish
checking in patches. This is the worst one, I promise!
Let me know what you want me to do, when you get a chance.
Thanks
----------------------------------------------------------------------
Comment By: Brad Clements (bkc)
Date: 2002-01-20 21:17
Message:
Logged In: YES
user_id=4631
I've eliminated Py_ClearErrno() and updated all the source
to use Py_SetErrno(0). Attached is an updated diff for
pyport.h
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-01-20 20:21
Message:
Logged In: YES
user_id=31435
Brad, errno is required by ANSI C, which also defines the
semantics of a 0 value. Setting errno to 0, and taking
errno==0 as meaning "no error", are 100% portable across
platforms with a standard-conforming C implementation. If
this platform doesn't support standard C, I have to
question whether the core should even try to cater to it:
the changes needed make no sense to C programmers, so may
become a maintenance nightmare.
I don't think putting a layer around errno is going to be
hard to live with, provided that it merely tries to emulate
standard behavior. For that reason, setting errno to 0 is
correct, but inventing a new ClearErrno concept is wrong
(the latter makes no sense to anyone except its inventor
<wink>).
----------------------------------------------------------------------
Comment By: Brad Clements (bkc)
Date: 2002-01-20 16:54
Message:
Logged In: YES
user_id=4631
I can post a new diff for the // or would you be willing to
just change the patch you have?
I cannot use the same macros for Py_SET_ERANGE_IF_OVERFLOW
(X) because Wince doesn't have ERANGE. You'll note the use
of Py_SetErrno(1) which is frankly bogus. This is related
to your comment on Py_ClearErrno()
Using (errno == 0) as meaning "no error" seems to me to be
a python source "convention" forced on it by (mostly)
floating point side effects. Because the math routines are
indicating overflow errors through the side effect of
setting errno (rather than returning an explicit NaN that
works on all platforms), we must set errno = 0 before
calling these math functions.
I suppose it's possible that on some platform "clearing the
last error value" wouldn't be done this way, but rather
might be an explicit function call. Since I was going
through the source looking for all errno's, I felt it was
clearer to say Py_ClearErrno() rather than Py_SetErrno(0),
even though in the end they do the same thing on currently
supported platforms.
I'm easy, if you want to replace Py_ClearErrno() with
Py_SetErrno(0) I can do that too.
--
Regarding goto targets.. is it likely that "cleanup" might
also collide with local variables? would _cleanup or
__cleanup work for you?
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-01-19 23:57
Message:
Logged In: YES
user_id=33168
Need to change the // comment to /* */. gcc accepts this
for C, but it's non-standard (at least it was, it may have
changed in C99).
You can have 1 Py_SET_ERANGE_IF_OVERFLOW for both platforms
if you do this:
#ifndef ERANGE
#define ERANGE 1
#endif
#define Py_SET_ERANGE_IF_OVERFLOW(X) \
do { \
if (Py_GetErrno() == 0 && ((X) == Py_HUGE_VAL || \
(X) == -Py_HUGE_VAL)) \
Py_SetErrno(ERANGE); \
} while(0)
I'm not sure of the usefulness of Py_ClearErrno(), since
it's the same on all platforms. If errno might be set to
something other than 0 in the future, it would be good to
make the change now.
I would suggest changing finally to cleanup.
----------------------------------------------------------------------
Comment By: Brad Clements (bkc)
Date: 2002-01-19 22:47
Message:
Logged In: YES
user_id=4631
Here is an amended diff with the suggested changes. I've
tested the semi-colon handling on EVT, it works as
suggested.
--
Question: What is the prefered style, #ifdef xyz or #if
defined(xyz) ?
I try to use #ifdef xyz, but sometimes there's multiple
possibilities and #if defined(x) || defined(y) is needed.
Is that okay?
--
Upcoming issue (hoping you address in your reply). There
are many "goto finally" statements in various modules.
Unfortunately EVT treats "finally" as a reserved word, even
when compiling in non C++ mode. Also, Metrowerks does the
same.
I've changed all of these to "goto my_finally" as a quick
work-around. I know "my_finally" sounds yucky, what's your
recommendation for this?
----------------------------------------------------------------------
Comment By: Tim Peters (tim_one)
Date: 2002-01-19 21:52
Message:
Logged In: YES
user_id=31435
All identifiers defined in pyport.h must begin with "Py_".
pyport.h is (and must be) #include'd by extension modules,
and we need the prefix to avoid stomping on their
namespace, and to make clear (to them and to us) that the
gimmicks are part of Python's portability layer. A name
like "SetErrno" is certain to conflict with some other
package's attempt to worm around errno problems; Py_SetErrno
() is not. Agree with Neal's final suggestion about
dealing with semicolons.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-01-19 21:28
Message:
Logged In: YES
user_id=33168
Typically, the semi-colon problem is dealt with as in
Py_SET_ERANGE_IF_OVERFLOW.
So,
#define SetErrno(X) do { SetLastError(X); } while (0)
I don't think (but can't remember if) there is any problem
for single statements like you have. You could probably do:
#ifndef MS_WINCE
#define SetErrno(X) errno = (X) /* note no ; */
#else
#define SetErrno(X) SetLastError(X) /* note no ; */
#endif
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=505846&group_id=5470