[PATCH] Fix up discard-const warnings in s4 and tries to remove "discard_const_p"s/"CONST DISCARD"s in both s3 and s4

Matthias Dieter Wallnöfer mwallnoefer at yahoo.de
Tue Sep 29 07:18:46 MDT 2009


Hi Volker,

Volker Lendecke schrieb:
> On Tue, Sep 29, 2009 at 02:56:18PM +0200, Matthias Dieter Wallnöfer wrote:
>   
>> Where exactly lies the problem here? Memory? (So I can understand the  
>> problem)
>>     
>
> For example this one: 
>
> diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
> index 4a338fa..eda1876 100644 (file)
> --- a/source3/smbd/msdfs.c
> +++ b/source3/smbd/msdfs.c
> @@ -1764,7 +1764,7 @@ NTSTATUS
> resolve_dfspath_wcard(TALLOC_CTX *ctx,
>                  * Once srvstr_get_path() uses talloc it'll
>                  * be a talloced ptr anyway.
>                  */
> -               *pp_name_out = CONST_DISCARD(char *,name_in);
> +               *pp_name_out = talloc_strdup(ctx, name_in);
>         }
>         return status;
>  }
>
> While probably the right thing to do, this introduces a bug:
> You don't check if the talloc_strdup actually worked. You
> need to check if talloc_strdup has returned NULL and in that
> case return NT_STATUS_NO_MEMORY from resolve_dfspath_wcard().
>   
Well - that's indeed correct. Will see to do some more investigation here.
> To be honest, I don't really like these mass-conversions if
> they are not done very, very carefully. This one I just
> picked is around in many places.
>
> Also, you need to make very certain that when you use a
> talloc_strdup or talloc_memdup that this is not in a hot
> codepath that might duplicate a lot of memory and thus
> create a performance hit. Also, you need to make very
> certain that you memdup/strdup on the right memory context.
> If by accident you put memory to a long-lived context, you
> have created a memory leak.
> Have you checked these points in all instances?
>   
Regarding the memory contexts : I did long checks  - that was much of my 
effort to use adequate memory contexts where they were possible. Where 
they weren't I didn't change anything (only if needed converted to the 
new macro) - the same with the not-trivial structures (since to perform 
there a deep copy isn't that simple - and as you said - the result could 
occupy much more memory).

Matthias
> Thanks,
>
> Volker
>
>   



More information about the samba-technical mailing list