Code Review request for new "jsign" tool

[Mandy Chung]

  On 05/10/11 10:10, Sean Mullan wrote:
> -f, --signedmodulefile <File: path>     File name of signed module file

This is a question more to the CLI guideline/convention our tools should 
follow.

Should an option name with multiple words uses '-' to separate each 
word?  "--signed-module-file" vs "--signedmodulefile"?

--output (-o) is the typical option name for specifying the output from 
a CLI.
Should we use --output for the jsign command?  I like the explicit
--signed-module-file option name.  It might be good to have a consistent
convention for all of our new tools to follow.

>
> webrev: 
> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/jsign/webrev.00/

Signer.java
     L255: what if the input module file is a signed module file? Should it
     output an error?

     L247, 248: constant 32 and 12 - is it worth defining these constants
     in the ModuleFileFormat class?

ModuleFileFormat.java
     L630-637:  I believe if noExtract is true, destination will be 
null; otherwise,
     destination is non-null.  Looks like you don't need the noExtract 
flag and
     perhaps replaced with needExtract method?  Just a thought.
     L642-643:  This assumes the 'sections' field doesn't count the 
signature
     section.  I assume you will take out the commented line before you push
     the push.

Other than that, looks good.

>
> I encountered one issue with the module-file format [2] which should 
> be addressed. Ideally, when a signature is generated over an existing 
> module file, none of the contents of that module file should be 
> modified. However, there is one field (the sections field in the 
> module file header) that breaks that rule, because the signature 
> itself is a section, and therefore the number of sections needs to be 
> incremented by one. It may be possible to do that, but it would result 
> in the code being much more complex. Thus, I would like to propose 
> that this field be changed to be the number of sections following the 
> module-info section (or the signature section if included), i.e. the 
> number of sections in the "rest" of the module. This would not affect 
> the Reader implementation, as it only uses this field to determine how 
> many sections it needs to read in the rest of the module.
>

Otherwise, you would need to rehash, right?  This sounds a good 
suggestion to me.

Mandy