s4: Patch for fixing LDB integer save operations on 64 bit platforms

[Matthias Dieter Wallnöfer]

Andrew,

thanks that you like my patch - I've added the comments and will push it 
soon. Then I will write a test to control the overflows.

Greets,
Matthias

Andrew Bartlett wrote:
> On Fri, 2010-10-15 at 21:08 +0200, Matthias Dieter Wallnöfer wrote:
>    
>> Okay, this is the latest patch proposal:
>> http://gitweb.samba.org/samba.git/?p=mdw/samba.git;a=commitdiff;h=26121b2ea1c7c6fce0589b1db89e385ea1367dd9
>>      
> The comment here is still quite incorrect.  There is nothing special
> about numbers>  2^16.  The issue is also not to do with nibble orders or
> big endian platforms.  As I see it, the issue is the behaviour of
> casting to 'unsigned' or 'signed' on platforms where a 'long' is 64 bits
> long.
>
> As I see it, your patch is correct, but we just need more comments -
> such as something like this on samldb_msg_add_uint() (because the
> current use is deceptive to the casual observer):
>
> -
>
> The issue here is that we have not yet first cast to int32_t explicitly,
> before we cast to an signed int to printf() into the %d or cast to a
> int64_t before we then cast to a long long to printf into a %lld.
>
> There are *no* unsigned integers in Active Directory LDAP, even the RID
> allocations and ms-DS-Secondary-KrbTgt-Number are *signed* quantities.
> (See the schema, and the syntax definitions in schema_syntax.c).
>
> -
>
> I think we should always cast to int32_t (but then again, if we are on a
> platform where 'int' isn't 32 bits, we are probably totally broken
> anyway, but it may still be safer).
>
> However, I do wonder if we are dealing with this at the wrong layer -
> you should first test if AD allows these large, signed numbers to be set
> over LDAP, and then what value is stored in the DB.  If the DB
> 'magically' wraps these to signed 32 bit numbers, then passing in large
> unsigned numbers is harmless, and we may simply need to work on our
> schema validation code instead.
>
> Finally, I do want to thank you for looking into this - issues of
> integer length can be tricky to debug, particularly when we have relied
> on implicit casts in printf().
>
> We should also remember that on databases other than sam.ldb, different
> rules may apply - they are not under such silly restrictions regarding
> unsigned integers.
>
> Andrew Bartlett
>