[Patches] [ python-Patches-1309352 ] Make fcntl work properly on AMD64

[SourceForge.net]

Patches item #1309352, was opened at 2005-09-30 03:18
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1309352&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Modules
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Submitted By: Brad Hards (bradh)
Assigned to: Neal Norwitz (nnorwitz)
Summary: Make fcntl work properly on AMD64

Initial Comment:
The  fcntl call doesn't work correctly on AMD-64, because of an 
unsigned int conversion problem. I found the problem using the 
dnotify.py code from buildbot.sf.net (attached). It (roughly) does: 
        self.flags = reduce(lambda x, y: x | y, flags) | 
fcntl.DN_MULTISHOT 
        self.fd = os.open(dirname, os.O_RDONLY) 
        fcntl.fcntl(self.fd, fcntl.F_NOTIFY, self.flags) 
 
fcntl.DN_MULTISHOT is 0x80000000, which causes 
OverflowError: signed integer is greater than maximum 
 
There is a similar fix already committed for ioctl - see 
http://cvs.sourceforge.net/viewcvs.py/python/python/dist/src/Modules/fcntlmodule.c?r1=2.43&r2=2.44 
 
 

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

>Comment By: Neal Norwitz (nnorwitz)
Date: 2005-10-11 21:45

Message:
Logged In: YES 
user_id=33168

Hmmm, how should we handle "Other values: implementation
defined"?  That really concerns me.  It seems that we can
never do the right thing, because we can't really know what
is right.

I can use I (unsigned int) format for the third parameter
which is a single char change from "i".  This fixes the
problem, but I'm not sure if it's best in the long run. 
Suggestions?

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

Comment By: Martin v. Löwis (loewis)
Date: 2005-10-11 16:58

Message:
Logged In: YES 
user_id=21627

POSIX/Single Unix has this signature of fcntl:

#include <fcntl.h>

int fcntl(int fildes, int cmd, ...);

The additional parameters depend on the cmd argument:
- F_DUPFD, F_SETFD, F_SETFL, F_SETOWN: int
- F_GETFD, F_GETFL, F_GETOWN, F_SETLKW: no argument
- F_GETLK, F_SETLK: struct flock*
Other values: implementation defined.

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

Comment By: Neal Norwitz (nnorwitz)
Date: 2005-09-30 20:33

Message:
Logged In: YES 
user_id=33168

Thanks for your patch.

I agree there's a problem, though I disagree with the fix. 
man fcntl says that the third argument is a long on my box
(gentoo).  Is that the same for you (3rd arg is long)?  I
don't think the first i (int arg) should be changed.  ISTM
only the second part of your patch (modifying the arg
variable) is on the right track.  I've attached my version
of the patch, can you test that this works for you?  It
seems to work for dnotify, though I'm not sure what it
should do.

Finally, could you create a real unittest in
Lib/test/test_fcntl.py?

Let me know if you agree or disagree with my assessment. 
Thanks.

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

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