Re: tools/widl

[Alexandre Julliard]

Ge van Geldorp  writes:
> -
>            switch(var->type->type)
> -
>            {
> -
>            case RPC_FC_BYTE:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_BYTE */\n", RPC_FC_BYTE);
> -
>                break;
> -
>            case RPC_FC_CHAR:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_CHAR */\n", RPC_FC_CHAR);
> -
>                break;
> -
>            case RPC_FC_WCHAR:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_WCHAR */\n", RPC_FC_WCHAR);
> -
>                break;
> -
>            case RPC_FC_USHORT:
> -
>            case RPC_FC_SHORT:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_USHORT */\n", RPC_FC_SHORT);
> -
>                break;
> -
>            case RPC_FC_ULONG:
> -
>            case RPC_FC_LONG:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_LONG */\n", RPC_FC_LONG);
> -
>                break;
> -
>            case RPC_FC_HYPER:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_HYPER */\n", RPC_FC_HYPER);
> -
>                break;
> -
>            case RPC_FC_IGNORE:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_IGNORE */\n", RPC_FC_IGNORE);
> -
>                break;
> -
>            case RPC_FC_SMALL:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_SMALL */\n", RPC_FC_SMALL);
> -
>                break;
> -
>            case RPC_FC_FLOAT:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_FLOAT */\n", RPC_FC_FLOAT);
> -
>                break;
> -
>            case RPC_FC_DOUBLE:
> -
>                print_client("0x4e,    /* FC_IN_PARAM_BASETYPE */\n");
> -
>                print_client("0x%02x,    /* FC_DOUBLE */\n", RPC_FC_DOUBLE);
> -
>                break;

You should add some sort of get_fc_name() function instead of
duplicating a big switch like that every time you want to print a
name.
> static void write_function_stubs(type_t *iface)
>  {
> - func_t *cur = iface->funcs;
> - char *handle_name = get_attrp(iface->attrs, ATTR_IMPLICIT_HANDLE);
> - char *implicit_handle = get_attrp(iface->attrs, ATTR_IMPLICIT_HANDLE);
> - int explitit_handle = is_attr(iface->attrs, ATTR_IMPLICIT_HANDLE);
> - func_t *func = iface->funcs;
> - var_t* var;

Your patch would be a lot easier to review if you avoided renaming
existing local variables. If you really think the names need to be
changed please do it in a separate patch.

tools/widl

[Ge van Geldorp]

> From: Alexandre Julliard
>
> You should add some sort of get_fc_name() function instead of 
> duplicating a big switch like that every time you want to 
> print a name.

This is a big problem for me. Eric Kohl (not me) did a lot of work on WIDL
in the ReactOS tree, there's about 20 patches to follow after this one. If
this first patch is not acceptable and needs to be rewritten, it means I can
no longer simply pull following patches from our SVN repository. Since I
didn't write the original code in the first place, having to do patch
generation manually will a) significantly increase the chance of introducing
bugs and b) take too much of my time.
Maybe we can try to find a solution at WineConf?

Ge van Geldorp.

tools/widl

[Alexandre Julliard]

"Ge van Geldorp"  writes:
> This is a big problem for me. Eric Kohl (not me) did a lot of work on WIDL
> in the ReactOS tree, there's about 20 patches to follow after this one. If
> this first patch is not acceptable and needs to be rewritten, it means I can
> no longer simply pull following patches from our SVN repository. Since I
> didn't write the original code in the first place, having to do patch
> generation manually will a) significantly increase the chance of introducing
> bugs and b) take too much of my time.
> Maybe we can try to find a solution at WineConf?

The solution is for ReactOS developers to submit Wine changes to
wine-patches at the same time as they commit them to the ReactOS tree,
instead of waiting for patches to accumulate and then trying to do a
big merge. That's how we do it at Codeweavers: we used to wait until
the next Crossover release to merge patches, but it was very painful,
so now developers submit changes to wine-patches as the same time as
they put them in the internal CVS, and it makes things a lot easier
for everybody.