Challenges in merging the charcnv code

[Andrew Bartlett]

On Thu, 2009-03-26 at 22:26 +0100, Volker Lendecke wrote:
> On Fri, Mar 27, 2009 at 08:18:01AM +1100, Andrew Bartlett wrote:
> > > To be honest, I'm too busy with other stuff to got through
> > > and put in the talloc_tos(). And I would have to understand
> > > all the push_string_fn macros which is quite intricate to
> > > me.
> > 
> > That leaves me rather confused:  Do you want this code merged or not? Do
> > you want me to do it?
> 
> As I said: The patches look good. But I would really have to
> take a lot of time to review it, this is just too central,
> and I would have to really learn about all the push_string
> macro magic. I stumbled over the very first patch of yours
> where you split up a function for two different use cases.
> 
> I might be too slow due to my age, but just from looking at
> the patch I could not quickly enough figure out what those
> two use cases are. So I would have to dive in an figure out
> what this is. Maybe I will do that over the weekend when no
> customers want me to do stuff, but I can't promise anything.

The two use cases are were we use 'base', and where we do not.

The base pointer points at the start of the SMB packet.  The comments
claimed that the pointer was used to determine if the packet was
unicode, but this was removed a while back (fortunately - replaced with
the flags2 parameter).  It is used to determine the alignment the string
should be on (assuming that it is possible - perhaps it is not - that
the start of the smb packet is on an odd alignment). 

Most of the cases do not use a base pointer, which is not provided by
the Samba4 charcnv API (which I was trying to get as close to as
possible).  These now call a simpler API (push_string_check - which
still checks buffer sizes using macro foo)

The remaining calls that did care about the base pointer are made via
other functions in clistr.c and srvstr.c, and so do not need a macro -
calling push_string_base directly.  

(Samba4 does not need the _base call because all smb buffers are
allocated with talloc(), and are therefore aligned.  I understand this
history in Samba3 comes from a time when smb packets were constructed on
the stack, and this may not apply in Samba3 either, but I was not
willing to make that call for this conversion). 

Finally, as to the macro renaming:  If we can avoid having a macro of
name 'push_string', then we can provide the Samba4 API as a normal
function, and my goal of having a common smbencrypt.c (yes, that was
what started all this) can be advanced.  

The pull_string macro was removed as it had no callers.

The patch stream does include a bug:  push_string_fn still references
flags2 in the first patch, and this is fixed in the last patch in the
series.  

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20090327/5fcf41cb/attachment.bin