"const" removal from OIDs (was "local memory contexts to avoid a discard_const_p()")

[Matthias Dieter Wallnöfer]

Is now everybody happy with my "oid_const" branch 
(http://gitweb.samba.org/samba.git/?p=mdw/samba.git;a=log;h=refs/heads/oid_const)? 
Especially Tridge?

Cheers,
Matthias

Matthias Dieter Wallnöfer wrote:
> Jelmer,
>
> I've uploaded a proposal: in my private repo on git.samba.org the 
> branch "oid_const".
>
> Cheers,
> Matthias
>
> Jelmer Vernooij wrote:
>> On Mon, 2010-12-13 at 10:55 +0000, Matthias Dieter Wallnöfer wrote:
>>
>>> but is there some particular reason that "oid" from
>>> "dsdb_schema_pfm_oid_from_attid" is returned "const" - as far as I've
>>> seen it's an allocated thing? If it's not then we would not need
>>> "discard_const_p" at all.
>> No, that indeed doesn't seem necessary. At first glance, changing that
>> function to return a non-const seems like a more reasonable approach. It
>> seems like it would also require changing some of the ber_read_*
>> functions though.
>>
>> Cheers,
>>
>> Jelmer
>>> On Sun, 2010-12-12 at 21:11 +0100, Jelmer Vernooij wrote:
>>>> Hi Matthias,
>>>>
>>>> As discussed earlier, can you please send this kind of patch to the
>>> list
>>>> for review first?
>>>>
>>>> I have my doubts about the usefulness of these changes in general
>>>> compared to the benefit. Eliminating talloc contexts doesn't really
>>> gain
>>>> us much. The overhead of creating an extra talloc context compared
>>> to
>>>> the overhead of using Python in the first place is negligible.
>>>>> @@ -248,27 +253,19 @@ static PyObject
>>> *py_dsdb_get_oid_from_attid(PyObject *self, PyObject *args)
>>>>>
>>>>>       PyErr_LDB_OR_RAISE(py_ldb, ldb);
>>>>>
>>>>> -    mem_ctx = talloc_new(NULL);
>>>>> -    if (mem_ctx == NULL) {
>>>>> -      PyErr_NoMemory();
>>>>> -      return NULL;
>>>>> -    }
>>>>> -
>>>>>       schema = dsdb_get_schema(ldb, NULL);
>>>>>
>>>>>       if (!schema) {
>>>>>           PyErr_SetString(PyExc_RuntimeError, "Failed to find a
>>> schema from ldb \n");
>>>>> -        talloc_free(mem_ctx);
>>>>>           return NULL;
>>>>>       }
>>>>>
>>>>>       status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap,
>>> attid,
>>>>> -                                            mem_ctx,&oid);
>>>>> +                                            NULL,&oid);
>>>>>       PyErr_WERROR_IS_ERR_RAISE(status);
>>>>>
>>>>>       ret = PyString_FromString(oid);
>>>>> -
>>>>> -    talloc_free(mem_ctx);
>>>>> +    talloc_free(discard_const_p(char, oid));
>>>> ^^ Is this really necessary? I'd rather have the extra memory
>>> context
>>>> than add an extra discard_const_p.
>>> I agree, it's really useful to be able to avoid the const issues by
>>> using a new memory context here.  Every discard_const_p() hides a
>>> potentially serious bug, and so should be avoided where other
>>> reasonable
>>> alternatives exist).  The local memory context seems a very reasonable
>>> alternative, and so it seems a backwards step to eliminate it.
>>>
>>> Andrew Bartlett
>>>
>>> -- 
>>> Andrew Bartlett
>>> http://samba.org/~abartlet/
>>> Authentication Developer, Samba Team          http://samba.org
>>> Samba Developer, Cisco Inc.
>>>
>>>
>
>