Bug 510711 - python: drop no longer needed canonicalize.patch
python: drop no longer needed canonicalize.patch
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: python (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: James Antill
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-10 07:44 EDT by Tomas Hoger
Modified: 2009-07-10 12:59 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 169046
Environment:
Last Closed: 2009-07-10 09:54:49 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tomas Hoger 2009-07-10 07:44:32 EDT
Unless I'm mistaken, we should no longer need python-2.*-canonicalize.patch that's being forward-ported since FC4 days.  It was added to address an issue reported in bug #169046, but never made it upstream.  The patch seems littler over-engineered and should no longer be needed, as I explain in:

  https://bugzilla.redhat.com/show_bug.cgi?id=169046#c8

I'd appreciate if someone can give it a second look and correct me if I'm wrong.


+++ This bug was initially created as a clone of Bug #169046 +++

I know this is ancient, but can anyone still remember further details about this?  I'd really love to understand why such a complex patch was created to address this, when one-liner as:

--- Python/sysmodule.c.canonicalize
+++ Python/sysmodule.c.fixed
@@ -1178,7 +1178,7 @@ void
 PySys_SetArgv(int argc, char **argv)
 {
 #if defined(HAVE_REALPATH)
-	char fullpath[MAXPATHLEN];
+	char fullpath[PATH_MAX];
 #elif defined(MS_WINDOWS)
 	char fullpath[MAX_PATH];
 #endif

should be good enough?  The problem here was that python was calling realpath with buffer of insufficient size.  The reason for that was that python defines MAXPATHLEN and does not use one defined on Linux in sys/param.h.  Python used to defined it unconditionally to 1024, regardless of the PATH_MAX value, which is the size expected by realpath.

The canonicalize.patch we use never made it upstream, but other changes resolved the issue there.  First, following change was done:

  http://svn.python.org/view?view=rev&revision=45715

MAXPATHLEN is no longer unconditionally set to 1024, but rather to PATH_MAX, if it's defined and is more than 1024.

Additionally, in python3, PATH_MAX is used as size of the buffer passed to realpath (see _wrealpath() in Python/sysmodule.c).
Comment 1 James Antill 2009-07-10 09:54:49 EDT
 Well I'm not desperate to do it "just because" ... but if it helps 482814 get done, then I don't see why we can't drop it as part of that fix (ie. the patch for 482814 would actually be "drop canonicalize, make path bigger and do it's own fix).
Comment 2 Tomas Hoger 2009-07-10 11:33:15 EDT
James, this is not about 482814.  I do understand that you are probably even lot more annoyed by 482814 than I am, but this is no blocker for that but, as that one can be fixed regardless of the canonicalization patch.

Yes, I came across that patch while looking at the other bug and it seemed rather useless to me, with no apparent signs of having a good chance to make it upstream.  So this was meant as "hey, we seem to have an extra patch that seem to have no real benefit, that we don't seem to be able to get accepted upstream.  let's have a look at the patch to see if we really want to carry it forward or can drop it safely now.".

Though you're the maintainer, if forward-porting sounds like a better plan, so be it.
Comment 3 James Antill 2009-07-10 12:59:28 EDT
Let me put it another way:

1. I really don't want to change anything in python we don't have to, for various reasons.

2. ...but if just rm'ing this patch makes anything else we want to do easier, I see no reason not to do it as part of that other fix.

Note You need to log in before you can comment on or make changes to this bug.