local memory contexts to avoid a discard_const_p()

[Kamen Mazdrashki]

Hi Matthias,

On Sat, Dec 18, 2010 at 11:58, Matthias Dieter Wallnöfer <mdw at samba.org> wrote:
> Hi Kamen,
>
> are you talking about this one:
> http://gitweb.samba.org/samba.git/?p=mdw/samba.git;a=commitdiff;h=712c96cad7cd2fbe3e5bff5be0b7388bd50159ae?
>
Yes, this one.
It changes prefixMap interface which I wanted to avoid for before
mentioned reason.

I was talking also about this commit:
http://git.samba.org/?p=samba.git;a=commitdiff;h=b974966c
And more specifically about this function: py_dsdb_get_oid_from_attid().

As far as I can understand, you've removed local mem_ctx and this
introduced the "discard_const_p" macro.
As I wrote before, I think this is wrong as you may make no assumptions
about the origin of the returned OID - it may ba allocated memory, but it
may be pointer to a const memory. Thats why I've made it const :)

> But do you not see the reason about removing "discard_const_p" uses? I think
> that's also a big argument to merge my patch. "discard_const_p" is ugly and
> it would disappear in the OIDs handling.
>
I can see the point in removing 'const' from ber_OID_* interface.
In my understanding ber_OID_* interface as buffer/stream decoding
interface is supposed to allocate output buffers. Thus we may assume
output is always allocated.
It is good to know what Tridge, as the author of this interface, has
in mind though.

> Cheers,
> Matthias
>
> Kamen Mazdrashki wrote:
>>
>> Hi Matthias,
>>
>> I think your patches are reasonable. If we are to remove consts from
>> ber_OID_* interface, then prefixMap implementation is affected too.
>> I have my doubts about prefixMap interface though.
>>
>> For instance, currently dsdb_schema_pfm_oid_from_attid() function
>> really allocates the returned OID on passed mem_ctx. My intention
>> though was that caller of this function not to make assumptions about
>> the OID buffer itself. I wanted the caller to assume this OID is a const
>> value - not to modify it for example.
>> I hope this way internal impelementation is more encapsulated. And
>> if we decide that ATTID-to-OID function is very expensive (giving the
>> fact it is frequently called), then we can implement a small cache
>> internally
>> and just return pointers to our internal cache (I realize that current
>> interface
>> doesn't make this intention obvious).
>>
>> To make long story short, I would really prefer for the prefixMap
>> interface
>> not to be changed at the moment.
>>
>> Now I see where all this started and I am really sorry I've missed missed
>> all
>> your discussions about py bindings. If it is about
>> py_dsdb_get_oid_from_attid()
>> implementation, then I think it is the py_dsdb_get_oid_from_attid() that
>> should
>> be fixed - it should have a temp memory context. First we should call
>> dsdb_get_schema(ldb, temp_ctx) - so that we are sure schema cache won't
>> disappear till the end of the function. Then we should call
>> dsdb_schema_pfm_oid_from_attid() again with our temp context. And finally,
>> free this temp_ctx and thus release both schema cache and any internal
>> allocations in dsdb_schema_pfm_oid_from_attid().
>>
>> In my opinion, this is a much safer implementation for
>> py_dsdb_get_oid_from_attid().
>>
>> Once again, sorry for missing the whole discussion.
>>
>>
>
>

Please see my previous e-mail that explains why I prefer prefixMap interface
not to be changed at the moment

-- 
Thanks,
Kamen Mazdrashki