Re: [PATCH] dbghelp: Make SymGetLineFromAddr64 call SymGetLineFromAddrW64

[Sebastian Lackner]

On 10.03.2017 04:24, Alistair Leslie-Hughes wrote:
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> ---
>  dlls/dbghelp/dbghelp_private.h |  3 ++
>  dlls/dbghelp/symbol.c          | 96 ++++++++++++++++++++++++++++++------------
>  2 files changed, 73 insertions(+), 26 deletions(-)

Could you give some explanation for the purpose of this patch? It looks a bit odd that
we need two almost identical symt_fill_func_line_info implementations.
> +        if (found)
> +        {
> +            char *filename = (char*)source_get(module, dli->u.source_file);
> +            int len = lstrlenA(filename)+1;
> +
> +            line->FileName = fetch_buffer(module->process, len * sizeof(WCHAR));
> +
> +            MultiByteToWideChar(CP_ACP, 0, filename, -1, line->FileName, len);
> [...]
> -    len = MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, NULL, 0);
> -    if ((l64w->FileName = fetch_buffer(pcs, len * sizeof(WCHAR))))
> -        MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, l64w->FileName, len);
> -    l64w->Address = l64->Address;

Actually, the old way of converting the strings looked more reasonable. Computing the
correct length and checking for errors is preferred in such cases.

Best regards,
Sebastian

[PATCH] dbghelp: Make SymGetLineFromAddr64 call SymGetLineFromAddrW64

[Alistair Leslie-Hughes]

On 11/03/17 03:26, Sebastian Lackner wrote:
> On 10.03.2017 04:24, Alistair Leslie-Hughes wrote:
> > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> > ---
> >  dlls/dbghelp/dbghelp_private.h |  3 ++
> >  dlls/dbghelp/symbol.c          | 96 ++++++++++++++++++++++++++++++------------
> >  2 files changed, 73 insertions(+), 26 deletions(-)
>
> Could you give some explanation for the purpose of this patch? It looks a bit odd that
> we need two almost identical symt_fill_func_line_info implementations.

The function stabs_finalize_function in stabs.c uses this function.
However updating this as well started a snowball if changes, making the
patch too large.  The symt_fill_func_line_info should be able to be
removed once the symt_get_func_line_next is updated to with a similar
change as this one.

Best Regards
Alistair