Issue 1699259: replacing char* with const char* in sysmodule.c/.h (original) (raw)

Created on 2007-04-12 13:55 by sebastinas, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
sysmodule_const_char.diff sebastinas,2007-04-12 13:55 patch for Include/sysmodule.h and Python/sysmodule.c review
sysmodule_const_char_r67215.diff sebastinas,2008-11-13 19:23 updated patch review
Messages (15)
msg52430 - (view) Author: Sebastian Ramacher (sebastinas) Date: 2007-04-12 13:55
I'm embedding Python in a C++ application and found myself casting const char* to char* quite often when using functions like PySys_SetObject and PySys_GetObject. I attached a patch replacing the char* arguments of the above mentioned functions, PySys_GetFile and PySys_AddWarnOption with const char*.
msg75812 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-11-13 04:26
The patch no longer applies to trunk, but that would be trivial to fix. Changes like these have been accepted with little resistance in the past (see e.g. ), so I don't see why this patch has been pending for so long. (In general, I am -0 on adding const modifiers because they tend to trigger an avalanche of changes, but arguably this is the "right thing to do.") A few comments: * Documentation needs to be updated in Doc/c-api/sys.rst. * Any reason why PySys_SetPath(char *) is left out? * Same for PySys_SetArgv(int, char **) * Similar changes are proposed in . Maybe these two issues can be dealt with together.
msg75833 - (view) Author: Sebastian Ramacher (sebastinas) Date: 2008-11-13 19:23
At least a response, finally. > * Any reason why PySys_SetPath(char *) is left out? I guess it I just missed it. > * Same for PySys_SetArgv(int, char **) That one is non-trivial and requires some rewriting of PySys_SetArgv. And I didn't have the time to look into it any further. I attached an updated patch.
msg75837 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-11-13 20:50
The new patch looks fine to me. It applies and compiles without warnings and the changes are now reflected in the docs. I guess someone would need to write a NEWS entry because it is a public API change, but otherwise I would say it should be applied. "[U]nless 'C' has adopted overloading while I wasn't looking ... it's hard to imagine what kind of problems you could introduce in `C' code by changing char* to char const* that wouldn't be caught at compile-time." (Dave Abrahams in C++-sig) http://mail.python.org/pipermail/cplusplus-sig/2005-December/009562.html On Thu, Nov 13, 2008 at 2:24 PM, Sebastian Ramacher <report@bugs.python.org> wrote: .. >> * Same for PySys_SetArgv(int, char **) > > That one is non-trivial and requires some rewriting of PySys_SetArgv. > And I didn't have the time to look into it any further. > I agree and I only tossed this one in to check if you are aware of the char ** issues. I believe it was agreed at some point that changing char ** to const char ** is not a good idea (see revision 42596 reverting such change), but I could not find a clear explanation of the associated problems. In any case, +1 from me in favor of applying this patch and I will try to add Jeremy Hylton to the "nosy" list since he is the developer who made similar API changes in r41638 .
msg75839 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-11-13 21:29
Sorry, but it's too late to apply the patch. The issues don't count as "critical" and it changes the API, too. Only critical and important bugs are solved during the release candidate phase of 3.0. Python 2.6 is already out. I set the target version to 2.7 and 3.1.
msg75844 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-11-13 23:45
While technically this is an API change, in reality it is unlikely to break anyone's code because you can always pass char * to a function that expects const char* and the ABI does not change. (Also I cannot think why anyone would want to use pointers to the affected functions, which would be another potential breakage.) In any case, I am not a big proponent of const correctness, but this patch was forgotten for 1.5 years and deferring it to 2.7 and 3.1 is virtually equivalent to closing with "won't fix". Is it clear that this patch is not a candidate for minor releases - 2.5.3 or 2.6.1? As I explained, it is not *really* an API change. If it is a backport candidate, I would see benefit in committing it sooner and blocking in py3k merge until 3.0 is out. On Thu, Nov 13, 2008 at 4:29 PM, Christian Heimes <report@bugs.python.org> wrote: > > Christian Heimes <lists@cheimes.de> added the comment: > > Sorry, but it's too late to apply the patch. The issues don't count as > "critical" and it changes the API, too. Only critical and important bugs > are solved during the release candidate phase of 3.0. Python 2.6 is > already out. > > I set the target version to 2.7 and 3.1. > > ---------- > stage: -> patch review > versions: +Python 2.7, Python 3.1 -Python 2.6 > > _______________________________________ > Python tracker <report@bugs.python.org> > <http://bugs.python.org/issue1699259> > _______________________________________ >
msg78795 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-02 14:03
Concerning the SetArgv( int, char**), that one will have to be changed to a SetArgv( int, char const* const*), i.e. applying the const on both levels. Otherwise, there is no implicit conversion between the two. The reason is a bit complicated: if the function stored a pointer-to-const in the array, it would become a pointer-to-nonconst outside. Only if modification of the array is forbidden, changing the element type to pointer-to-const is allowed. Otherwise, I'm +1 on this kind of changes, because it makes immediately obvious which functions modify a passed buffer and which don't.
msg92943 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-09-21 14:08
See also issue 6952, which seems to be a broader request of the same nature. Moving the 3.1 target to 3.2, since 3.1 is out.
msg110578 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-17 16:32
Drop 2.7 as that's now gone. See also #6952.
msg110604 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-17 21:28
2.7 is not gone. It has entered an extended maintenance period. During this period, 2.7.x releases will incorporate bug fixes but will not get any new features. This particular issue is arguably a bug. I think Christian should make a call on whether this is appropriate for 2.7.
msg110618 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-17 23:52
This is quite clearly marked as a feature request and hence is 3.2. If you wish to change it go ahead, but who's going to do the work?
msg119027 - (view) Author: Sebastian Ramacher (sebastinas) Date: 2010-10-18 15:17
Any news on that?
msg119031 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-18 15:34
On Mon, Oct 18, 2010 at 11:17 AM, Sebastian Ramacher <report@bugs.python.org> wrote: .. > Any news on that? Is this patch still relevant for 3.2? It looks like const has been added when char* was changed to wchar_t* in the affected functions. See r62178.
msg119032 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-10-18 15:44
In fact, it looks like const has been added in py3k as early as r57439. I am resetting "versions" to 2.7, but I am -0 on backporting. I am also unselecting "type" because with "feature request" type this should be closed while some may argue that this is a bug.
msg137685 - (view) Author: Sebastian Ramacher (sebastinas) Date: 2011-06-05 07:12
Since the patches are not applicable to Py 3.x and Py 2.7 is stable I'm closing this bug.
History
Date User Action Args
2022-04-11 14:56:23 admin set github: 44837
2011-06-05 07:12:09 sebastinas set status: open -> closedresolution: out of datemessages: +
2010-10-18 15:44:41 belopolsky set versions: + Python 2.7, - Python 3.2nosy: - BreamoreBoymessages: + type: enhancement ->
2010-10-18 15:34:13 belopolsky set messages: +
2010-10-18 15:17:15 sebastinas set messages: +
2010-07-17 23:52:30 BreamoreBoy set messages: +
2010-07-17 21:28:29 belopolsky set messages: +
2010-07-17 16:32:28 BreamoreBoy set nosy: + BreamoreBoymessages: + versions: - Python 2.7
2009-09-21 14:08:01 r.david.murray set nosy: + r.david.murraymessages: + versions: + Python 3.2, - Python 3.1
2009-05-15 01:51:24 ajaksu2 link issue1772673 dependencies
2009-01-02 14:03:38 eckhardt set nosy: + eckhardtmessages: +
2008-11-13 23:45:50 belopolsky set messages: +
2008-11-13 21:29:35 christian.heimes set stage: patch reviewmessages: + versions: + Python 3.1, Python 2.7, - Python 2.6
2008-11-13 20:53:54 belopolsky set nosy: + jhylton
2008-11-13 20:51:00 belopolsky set messages: +
2008-11-13 19:23:58 sebastinas set files: + sysmodule_const_char_r67215.diffmessages: +
2008-11-13 04:26:03 belopolsky set nosy: + belopolskymessages: +
2008-01-06 11:58:13 christian.heimes set assignee: christian.heimestype: enhancementnosy: + christian.heimesversions: + Python 2.6
2007-04-12 13:55:10 sebastinas create