[Patches] [ python-Patches-434992 ] Cleanup of warning messages
noreply@sourceforge.net
noreply@sourceforge.net
Thu, 19 Jul 2001 15:03:12 -0700
Patches item #434992, was opened at 2001-06-20 18:51
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=434992&group_id=5470
Category: None
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Robert Minsk (rminsk)
Assigned to: Fred L. Drake, Jr. (fdrake)
Summary: Cleanup of warning messages
Initial Comment:
I just compiled Python-2.1 of the SGI using the latest
compilers (7.3.1.2m) with all the warning flags turned
on. The following patch will get rid of most of the
warning messages. I would like to see this
incorporated into the next release. It is easier to
spot real problems when you do not have to sort thru
other warning messages.
The included patch does not include other optional
modules and the ones
setup.py finds by default.
I may have found 2 bugs in the process. Please see
bugs 434989 and
434988.
----------------------------------------------------------------------
>Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-07-19 15:03
Message:
Logged In: YES
user_id=3066
The rest of the patches don't appear to apply worth anything any more (we've had an active month in the source tree!), so I'm closing this as "partly accepted". If there are any remaining warnings on the SGI, please submit a fresh patch, and send me an email so it doesn't go stale before I get to it.
Thanks for all the work!
----------------------------------------------------------------------
Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-07-19 14:50
Message:
Logged In: YES
user_id=3066
Only a couple of patches still make sense in the Objects/
directory:
fileobject.c 2.114
intobject.c 2.59
----------------------------------------------------------------------
Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-07-19 14:32
Message:
Logged In: YES
user_id=3066
I've checked in a lot of the patches here in the Modules/
directory:
_cursesmodule.c 2.54
pcremodule.c 2.28
posixmodule.c 2.194
selectmodule.c 2.53
signalmodule.c 2.59
socketmodule.c 1.151
unicodedata.c 2.13xreadlinesmodule.c 1.7
Some patches from the Modules/ directory appear to no longer
apply. Please file a separate patch if anything additional
is needed in the Modules/ directory.
Starting on the rest of the patch. (Breaking this up more
would have been good.)
----------------------------------------------------------------------
Comment By: Fred L. Drake, Jr. (fdrake)
Date: 2001-07-18 14:55
Message:
Logged In: YES
user_id=3066
Note that the separate cPickle has been integrated.
Assigning this to me since I handled that one.
----------------------------------------------------------------------
Comment By: Robert Minsk (rminsk)
Date: 2001-06-25 17:03
Message:
Logged In: YES
user_id=132786
> I'm not sure these patches are all correct. For the
> patches introducing prototypes (e.g. tigetstr), isn't
> there some header file that offers these prototypes?
That is what my patch fixed. It was changing from
#ifdef sgi
extern char *tigetstr(char *);
extern char *tparm(char *instring, ...);
#endif
to
#ifdef __sgi
#include <term.h>
#endif
> For cPickle, looking up string_atol seems to be completely
> unneeded. In turn, looking up string is unneeded, as well.
> Likewise, don't just remove empty_str, remove the lookup
> as well.
Are you saying remove the
UNLESS (PyString_FromString("")) return -1;
also. I guess I missed that.
> I think p should be unsigned
> char*, and the casts should then be adjusted to unsigned.
Should I fix that or are you looking into it?
> Why is it necessary to cast the result of umask? Please
> put a comment in the code, explaining that in detail (i.e.
> "required for SGI" is not sufficient).
Even on linux umask returns the type umask_t which may not
be
an int. I could change the code to
int i;
umask_t u;
if (!PyArg_ParseTuple(args, "i:umask", &i))
return NULL;
u = umask((mask_t)i);
but is umask_t available on all machines?
This is not a critical warning, in fact on the SGI it is
only when you compile with -fullwarn and it's only an INFO
message. The INFO messages are useful to
identifiy potential errors. The casts should not add any
overhead. This is one reason you should compile code on
multiple compilers. Each compiler has it's own strength and
weaknesses at identifing problems. This is not required for
SGI but just to clean up messages from other compilers
besides gcc. Other vendors compilers also give other
warning messages.
> Likewise for
> alarm
> (which returns long on Linux), and all other casts that
> were introduced to convert system call results or
> arguments.
Linux (at least RedHat 6.2) does not return a long from
alarm, it returns an unsigned int. Should I change the
signal_alarm to PyInt_FromUnsignedLong(alarm(t))?
Are there other platforms that return a signed long from
alarm? I would rather cast
to the type the function currently uses.
The cast are just casting to the type the functions expect.
This goes on if not explicity cast anywhy. So why not get
rid of the implicit cast.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2001-06-23 13:25
Message:
Logged In: YES
user_id=21627
I'm not sure these patches are all correct. For the
patches introducing prototypes (e.g. tigetstr), isn't
there some header file that offers these prototypes?
For cPickle, looking up string_atol seems to be completely
unneeded. In turn, looking up string is unneeded, as well.
Likewise, don't just remove empty_str, remove the lookup
as well.
On the save_float changes, you mask a range error: the
values will be in 0..255, but you cast this value to char,
which is potentially signed. I think p should be unsigned
char*, and the casts should then be adjusted to unsigned.
Since cPickle changes will need careful review, I
recommend to submit them as a separate patch.
Why is it necessary to cast the result of umask? Please
put a comment in the code, explaining that in detail (i.e.
"required for SGI" is not sufficient). Likewise for alarm
(which returns long on Linux), and all other casts that
were introduced to convert system call results or
arguments.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2001-06-23 13:00
Message:
Logged In: YES
user_id=21627
Refiled as a patch.
----------------------------------------------------------------------
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=434992&group_id=5470