Re: msasn1: implement ASN1_CreateModule

[Nikolay Sivov]

On 27.10.2015 23:10, Austin English wrote:
> This is based on code from the FreeRDP project, used with permission
> of the author, Marc-André Moreau

Do you plan to reuse more from there? Is it clean license-wise? And more 
importantly, how did FreeRDP project came up with that implementation?
> For https://bugs.winehq.org/show_bug.cgi?id=38020
> -
>
> +ASN1module_t WINAPI ASN1_CreateModule(ASN1uint32_t version, ASN1encodingrule_e rule, ASN1uint32_t flags, ASN1uint32_t pdu, const ASN1GenericFun_t encoder[], const ASN1GenericFun_t decoder[], const ASN1FreeFun_t freememory[], const ASN1uint32_t size[], ASN1magic_t modulename)

arg[] notation seems redundant, *arg is cleaner and more common for Wine.
> - ASN1module_t module = NULL;

No need to init this.
> - if (!((encoder) && (decoder) && (freememory) && (size)))
> -
>    return NULL;

...
> - module = (ASN1module_t) malloc(sizeof(struct tagASN1module_t));
> - ZeroMemory(module, sizeof(struct tagASN1module_t));

Heap* API please. And you don't need to zero-init it, or use tag* names.

All structures seem to be documented well, what's a reason to reuse 
simple parts like such structure allocations?

Can this be tested by directly calling ASN1_* functions? If yes, simple 
test would be nice to have.

msasn1: implement ASN1_CreateModule

[Austin English]

On Tue, Oct 27, 2015 at 3:29 PM, Nikolay Sivov  wrote:
> On 27.10.2015 23:10, Austin English wrote:
> > This is based on code from the FreeRDP project, used with permission
> > of the author, Marc-André Moreau
>
> Do you plan to reuse more from there? Is it clean license-wise? And more
> importantly, how did FreeRDP project came up with that implementation?

I have no plans for that currently, FoxIt didn't use any other
functions that were implemented in FreeRDP. Stubs were sufficient for
those.

The licenses are different, but Marc agreed to relicense under LGPL 2.1:
Hi Austin,

You can go ahead and use my asn1 sources from FreeRDP/WinPR under the
LGPL2.1 license. I am the only author for that module, and I am giving
you a copy of it under the LGPL2.1 license as of today. The mention
you intend to put in the source code should suffice to avoid
confusion.

Let me know if you need pointers on how to continue the work on
reversing msasn1.dll. Stubbing the headers took a while, you at least
have that already done for you :)

Best regards,
-Marc-Andre

I don't know the details for how FreeRDP implemented it.
> > For https://bugs.winehq.org/show_bug.cgi?id=38020
> > -
> >
> > +ASN1module_t WINAPI ASN1_CreateModule(ASN1uint32_t version,
> > ASN1encodingrule_e rule, ASN1uint32_t flags, ASN1uint32_t pdu, const
> > ASN1GenericFun_t encoder[], const ASN1GenericFun_t decoder[], const
> > ASN1FreeFun_t freememory[], const ASN1uint32_t size[], ASN1magic_t
> > modulename)
>
> arg[] notation seems redundant, *arg is cleaner and more common for Wine.
> > - ASN1module_t module = NULL;
>
> No need to init this.
> > - if (!((encoder) && (decoder) && (freememory) && (size)))
> > -
> >    return NULL;
>
> ...
> > - module = (ASN1module_t) malloc(sizeof(struct tagASN1module_t));
> > - ZeroMemory(module, sizeof(struct tagASN1module_t));
>
> Heap* API please. And you don't need to zero-init it, or use tag* names.

OK.
> All structures seem to be documented well, what's a reason to reuse simple
> parts like such structure allocations?
>
> Can this be tested by directly calling ASN1_* functions? If yes, simple test
> would be nice to have.

I'll test.

msasn1: implement ASN1_CreateModule

[Dmitry Timoshkov]

Austin English  wrote:
> Let me know if you need pointers on how to continue the work on
> reversing msasn1.dll.

This sentence doesn't sound like a good reason for using the FreeRDP sources.