[Samba] iconv.c / static charset prototype/assembler bug?

[Jeremy Allison]

On Tue, Jun 27, 2006 at 02:25:28PM +0200, Robert Szeleney wrote:
> Hi!
> 
> I think there is a little bug in the current samba release (3.0.22).
> 
> Take a look at cp850.c at the last line and you will see following 
> definition:
> 
>         SMB_GENERATE_CHARSET_MODULE_8_BIT_GAP(CP850)
> 
> Using the macros from charset.c, the preprocessor expands this to:
> 
>         NTSTATUS charset_CP850_init(void) \
>         {       \
>                 return smb_register_charset(&CP850_functions);          \
>         }       \
> 
> NTSTATUS is defined in nt_status.h as follows:
>  
>         typedef struct {uint32 v;} NTSTATUS;
>         #define NT_STATUS(x) ((NTSTATUS) { x })
>         #define NT_STATUS_V(x) ((x).v)
> 
> 
> Ok, when cp850/cp437 is compiled as static, config.h has following macro:
>         #define static_init_charset { charset_CP850_init(); 
> charset_CP437_init();} 
> 
> This static_init_charset macro gets called in iconv.c. But at this point 
> in the file there is no function prototype for charset_CP850_init and 
> charset_CP437_init. Thus gcc, doesn't know that it has to reserve space on 
> the stack for this 4 byte return value which actually gets returned AND 
> pushed to the stack by charset_CP850_init.

Ok, I've looked into this in the current SAMBA_3_0 codebase
(the code that's in 3.0.23RC3) and this problem shouldn't
happen.

When you configure with :

./configure.developer --with-static-modules=charset_CP850

You get :

/* Decl of Static init functions */
#define static_decl_charset extern NTSTATUS charset_CP850_init(void);

followed by :

#define static_init_charset {  charset_CP850_init();}

defined in include/config.h

in lib/iconv.c we have :

static_decl_charset;

at the top of the file (after #include "includes.h")
followed by :

static void lazy_initialize_iconv(void)
{
        static BOOL initialized;
        int i;

        if (!initialized) {
                initialized = True;
                for(i = 0; builtin_functions[i].name; i++)
                        smb_register_charset(&builtin_functions[i]);
                static_init_charset;
        }
}

- this should cause the correct declaration and definition to
be used. Follow up this email if you think I'm incorrect on
this but I don't think it's a problem in 3.0.23 RC3.

Jeremy.