[PATCH] Change unix_convert to use struct smb_filename

[Jeremy Allison]

On Thu, Apr 30, 2009 at 12:02:32PM -0700, Tim Prouty wrote:
>
> Thanks for the feedback!
>
> I was considering that option, but since we already had a talloc_ctx  
> being passed around, it seemed simpler to just have the smb_filename  
> struct sit on the stack.  I guess one possible advantage of tallocing an 
> smb_filename struct is that the talloc_ctx arg could be eliminated from 
> unix_convert, further simplifying the API.

Actually you can't do that as you sometimes might want to
talloc one *not* on talloc_tos() (which I'm assuming you
were intending using instead - using the NULL context is
not recommended). So you still need the talloc_ctx argument
to unix_convert() even in the talloc'ed struct smb_filename
returned case.

> Another advantage is that the memory would be more cleanly tracked, and 
> could be freed earlier as soon as it is no longer being used.  Are there 
> other advantages as well?
>
> On a related note, in a future patch it would make sense to store the  
> smb_filename struct in the file_struct as well.  To add this ability  
> I'll probably also add some utility functions that alloc/init/copy/etc  
> smb_filename structs.  These utility structs could be used here as well.

Ok, I went over this patch and I really like it (although
it won't apply with git am to master anymore :-).

I agree with Volker. unix_convert should return a talloc'ed
struct smb_filename instead of being passed one, and the
pointer elements of struct smb_filename should be talloc'ed
as children of the returned struct (much easier cleanup).

With that change I'm really happy for this to go in.

> I'm not familiar with the talloc_pool trick.  Are there examples of the 
> trick in use elsewhere?

It's used in the initial talloc frame allocated to process
incoming SMB requests. See talloc_stackframe_pool() in smbd/process.c.
It creates a pool of 8192 bytes which can be allocated from
by simply incrementing a pointer instead of doing a new malloc.

If the allocated struct smb_filename and associated elements
become a bottleneck we'd use it here, but almost certainly
we won't need it as the allocated struct smb_filename will
already fit in the initial 8k talloc_pool stackframe for
processing incoming requests.

Jeremy.