[PATCH] Change unix_convert to use struct smb_filename
jra at samba.org
Fri May 1 00:19:00 GMT 2009
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
> 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.
More information about the samba-technical