UTF-8 and LDAP

Andrew Bartlett abartlet at pcug.org.au
Sat Mar 2 15:13:02 GMT 2002


Juergen Hasch wrote:
> 
> Am Samstag, 2. März 2002 01:59 schrieb Andrew Bartlett:
> >
> > This looks the prefereable choice, but use the existing
> > 'convert_string_allocate' function.
> >...
> > The existing function is a bit better with memory allocation, but thats
> > the basic idea.  If you want to add a 'convert_string_talloc' it would
> > probably be quite useful, but otherwise, malloc()/SAFE_FREE() should be
> > fine.
> 
> OK, I guess this is more like it. I added a convert_string_talloc() to
> charcnv.c and use either convert_string_talloc() or convert_string_allocate()
> directly in ldap.c.

Did you look at the existing convert_string_allocate() function before
you started?

Being a multibyte charset, a UTF8 conversion can potentially expand the
string by >2 times.

I suggest you implement convert_string_talloc() in by using
convert_string_alloc() and then just talloc_strdup()/SAFE_FREE() the
returned value.

Also, you are not checking the convert_string_talloc return values in
ldap.c, only the passed paramater.  It is a really bad idea to assume
that this paramater has any meaning if a function fails.  (I know its
fine for your function, but its not a good idea in general).

You should document all the paramaters, in particular the 'aways sets
dest to at least null' behaviour in the doxygen comment above the
function.

Finally, wrapping these in a pull_utf8_talloc() and push_utf8_talloc()
would make the call clearer, and allow the use of -1 as a 'length'
again.  (This would bring the code back to looking like the original
patch, but using allocated strings).

Hope this helps,

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net




More information about the samba-technical mailing list