local memory contexts to avoid a discard_const_p()

Matthias Dieter Wallnöfer mdw at samba.org
Sun Dec 19 04:52:20 MST 2010


Hi Kamen,

so I will:
- remove this commit: 
http://gitweb.samba.org/samba.git/?p=mdw/samba.git;a=commitdiff;h=712c96cad7cd2fbe3e5bff5be0b7388bd50159ae?
- restore the "mem_ctx" in "pydsdb.c" (py_dsdb_get_oid_from_attid())

Just to doublecheck - is this what you want?

Cheers,
Matthias

Kamen Mazdrashki wrote:
> 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
>
>    



More information about the samba-technical mailing list