993489 - ECC decode refactoring needed to build OpenJDK SunEC provider for ECC support

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Elio Maldonado]

OpenJDK's SunEC provider currently suppports ecc via duplicated code from NSS and they need to have the SunEC provider use the system NSS instead. More details can be found on https://bugzilla.redhat.com/show_bug.cgi?id=1075702.
After Bob's analysis this is best accomplished by ecc decode refactoring that moves it from softoken to freebl.

Comment 1

[Elio Maldonado]

Attachment: Refactor ecdecode from softoken to freebl

Comment 3

[Wan-Teh Chang]

Comment on attachment 8403404 [details] [diff] [review]
Refactor ecdecode from softoken to freebl

Review of attachment 8403404 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/freebl/ecdecode.c
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

Please make sure you use the right hg command to move this file,
so that the revision history is preserved.

::: lib/freebl/loader.h
@@ +710,2 @@
>    /* Add new function pointers at the end of this struct and bump
>     * FREEBL_VERSION at the beginning of this file. */

Please bump REEBL_VERSION at the beginning of this file.

::: lib/freebl/stubs.c
@@ +522,5 @@
> +extern SECOidTag
> +SECOID_FindOIDTag_stub(const SECItem *oid)
> +{
> +    STUB_SAFE_CALL1(SECOID_FindOIDTag_Util, oid);
> +    abort();

Should we return something to avoid a compiler warning?

Comment 4

[Elio Maldonado]

(In reply to Wan-Teh Chang from comment #3)
> Comment on attachment 8403404 [details] [diff] [review]
> Refactor ecdecode from softoken to freebl
> 
> Review of attachment 8403404 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: lib/freebl/ecdecode.c
> @@ +1,1 @@
> > +/* This Source Code Form is subject to the terms of the Mozilla Public
> 
> Please make sure you use the right hg command to move this file,
> so that the revision history is preserved.
> 

I think the proper hg command is hg rename lib/softokn/ecdecode.c lib/freebl/ecdecode.c

> ::: lib/freebl/loader.h
> @@ +710,2 @@
> >    /* Add new function pointers at the end of this struct and bump
> >     * FREEBL_VERSION at the beginning of this file. */
> 
> Please bump REEBL_VERSION at the beginning of this file.
That would be from 0x0310 to 0x0311, isn't it?

> 
> ::: lib/freebl/stubs.c
> @@ +522,5 @@
> > +extern SECOidTag
> > +SECOID_FindOIDTag_stub(const SECItem *oid)
> > +{
> > +    STUB_SAFE_CALL1(SECOID_FindOIDTag_Util, oid);
> > +    abort();
> 
> Should we return something to avoid a compiler warning?

Yes, I think we should return SEC_OID_UNKNOWN.

Comment 5

[Wan-Teh Chang]

Comment on attachment 8403404 [details] [diff] [review]
Refactor ecdecode from softoken to freebl

Review of attachment 8403404 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/freebl/loader.h
@@ +710,2 @@
>    /* Add new function pointers at the end of this struct and bump
>     * FREEBL_VERSION at the beginning of this file. */

FREEBL_VERSION should be changed from 0x0310 to 0x0311. (0x11 is 17 decimal.)

Comment 6

[Elio Maldonado]

Attachment: Refactor eccdecode from softoken to freebl - V2

Address wtc's review comments. It lacks white space clean up.

Comment 7

[Elio Maldonado]

Attachment: Refactor eccdecode from softoken to freebl - V3

Like V2 but also cleaned up white space.

Comment 8

[Elio Maldonado]

Pushed: https://hg.mozilla.org/projects/nss/rev/4ed7ac939ad7

Comment 9

[Elio Maldonado]

Previous commit had the 'hg rename' missing for some reason, i'm still new to the ways or mercurial
Added the missing part: https://hg.mozilla.org/projects/nss/rev/4a6b3e8efd9f

Comment 10

[Elio Maldonado]

I had to back out the last two changesetss. They don't do what I expected.

Comment 11

[Elio Maldonado]

Pushed: http://hg.mozilla.org/projects/nss/rev/f3e3b8b185fa

Comment 12

[Elio Maldonado]

(In reply to Elio Maldonado from comment #4)
> (In reply to Wan-Teh Chang from comment #3)
> > Comment on attachment 8403404 [details] [diff] [review]
> > Refactor ecdecode from softoken to freebl
> > 
> > Review of attachment 8403404 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: lib/freebl/ecdecode.c
> > @@ +1,1 @@
> > > +/* This Source Code Form is subject to the terms of the Mozilla Public
> > 
> > Please make sure you use the right hg command to move this file,
> > so that the revision history is preserved.
> > 
> 
> I think the proper hg command is hg rename lib/softokn/ecdecode.c
> lib/freebl/ecdecode.c

Thatt's not enough and had to do an explicit add and remove but the history is indeed preserved. An hg log lib/freebl/ecdecode.c shows you only the last commit but you can still see the old history in the old directory via hg lib/softoken/ecdecode.c even though the file has been moved.