Replace kind variable from validator API with Boolean stubout parameter.

Replace kind variable from validator API with Boolean stubout parameter.

There were actually three kinds, but the NaClApplyValidationAnnotator kind was unused. 

This is an incremental step in cleaning up the validator API.

BUG= none
TEST= none

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=8219

[Mark Seaborn, 2012-04-10 00:57:38]

https://chromiumcodereview.appspot.com/10024039/diff/1/src/tools/validator_to...
File src/tools/validator_tools/ncstubout.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/tools/validator_to...
src/tools/validator_tools/ncstubout.c:42: (sb_kind, load_address, code,
code_size, bundle_size, TRUE, FALSE,
Can you write it as
/* stubout_mode= */ TRUE,
/* readonly_text= */ FALSE
for readability?

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/service_ru...
File src/trusted/service_runtime/sel_validate_image.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/service_ru...
src/trusted/service_runtime/sel_validate_image.c:89: /* fixed feature cpu mode
implies read-only*/
Space before '*/'

I'd split this into a local var:
  /* ...implies read-only... */
  bool readonly_text = nap->fixed_feature_cpu_mode;

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
File src/trusted/validator/ncvalidate.h (left):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/ncvalidate.h:77: * been stubbed out. It is the
responsability of the caller to
This commentary seems to be lost in this change.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/ncvalidate.h:93: NaClApplyValidationAnnotator
If this is unused, it helps to say that in the commit message.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
File src/trusted/validator/validation_cache_test.cc (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/validation_cache_test.cc:138: bundle_size, FALSE, FALSE,
&cpu_features,
Could you add comments for the args here too, perhaps?

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
File src/trusted/validator_x86/ncenuminsts_x86_32.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
src/trusted/validator_x86/ncenuminsts_x86_32.c:108: FALSE, FALSE, &cpu_features,
NULL);
Again, labelling the args would be good for readability.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
File src/trusted/validator_x86/ncenuminsts_x86_64.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
src/trusted/validator_x86/ncenuminsts_x86_64.c:169: FALSE, FALSE, &cpu_features,
NULL);
Ditto

[pasko-google - do not use, 2012-04-10 05:23:24]

Can you, please, also (in the future changes) remove the bundle_size argument?
It was an early experimental feature that has never been working in production,
the toolchain removed the command-line flag responsible for generating 16-byte
bundles long time ago.

[Nick Bray (chromium), 2012-04-10 18:49:20]

PTAL

pasko: bundle size elimination is planned for CL 3 out of 4(?)

https://chromiumcodereview.appspot.com/10024039/diff/1/src/tools/validator_to...
File src/tools/validator_tools/ncstubout.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/tools/validator_to...
src/tools/validator_tools/ncstubout.c:42: (sb_kind, load_address, code,
code_size, bundle_size, TRUE, FALSE,
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> Can you write it as
> /* stubout_mode= */ TRUE,
> /* readonly_text= */ FALSE
> for readability?

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/service_ru...
File src/trusted/service_runtime/sel_validate_image.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/service_ru...
src/trusted/service_runtime/sel_validate_image.c:89: /* fixed feature cpu mode
implies read-only*/
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> Space before '*/'
> 
> I'd split this into a local var:
>   /* ...implies read-only... */
>   bool readonly_text = nap->fixed_feature_cpu_mode;

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
File src/trusted/validator/ncvalidate.h (left):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/ncvalidate.h:77: * been stubbed out. It is the
responsability of the caller to
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> This commentary seems to be lost in this change.

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/ncvalidate.h:93: NaClApplyValidationAnnotator
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> If this is unused, it helps to say that in the commit message.

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
File src/trusted/validator/validation_cache_test.cc (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator/...
src/trusted/validator/validation_cache_test.cc:138: bundle_size, FALSE, FALSE,
&cpu_features,
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> Could you add comments for the args here too, perhaps?

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
File src/trusted/validator_x86/ncenuminsts_x86_32.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
src/trusted/validator_x86/ncenuminsts_x86_32.c:108: FALSE, FALSE, &cpu_features,
NULL);
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> Again, labelling the args would be good for readability.

Done.

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
File src/trusted/validator_x86/ncenuminsts_x86_64.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/1/src/trusted/validator_...
src/trusted/validator_x86/ncenuminsts_x86_64.c:169: FALSE, FALSE, &cpu_features,
NULL);
On 2012/04/10 00:57:38, Mark Seaborn wrote:
> Ditto

Done.

[Mark Seaborn, 2012-04-10 19:04:09]

LGTM

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/service_r...
File src/trusted/service_runtime/sel_validate_image.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/service_r...
src/trusted/service_runtime/sel_validate_image.c:82: /* fixed feature cpu mode
implies read-only */
Capitalise 'Fixed' and 'CPU'

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
File src/trusted/validator/ncvalidate.h (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/ncvalidate.h:77: *    stubout_mode - If the validator
should stub out illegal instructions.
'If' -> 'Whether'

'illegal' -> 'forbidden'/'disallowed'

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/ncvalidate.h:82: *           the responsability of the
caller to call the validator a second
'responsibility'

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
File src/trusted/validator/x86/32/ncvalidate.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/x86/32/ncvalidate.c:111: guest_addr, data, size,
bundle_size,
Nit: indent -2

[Nick Bray (chromium), 2012-04-10 19:12:40]

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/service_r...
File src/trusted/service_runtime/sel_validate_image.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/service_r...
src/trusted/service_runtime/sel_validate_image.c:82: /* fixed feature cpu mode
implies read-only */
On 2012/04/10 19:04:09, Mark Seaborn wrote:
> Capitalise 'Fixed' and 'CPU'

Done.

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
File src/trusted/validator/ncvalidate.h (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/ncvalidate.h:77: *    stubout_mode - If the validator
should stub out illegal instructions.
On 2012/04/10 19:04:09, Mark Seaborn wrote:
> 'If' -> 'Whether'
> 
> 'illegal' -> 'forbidden'/'disallowed'

Done.

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/ncvalidate.h:82: *           the responsability of the
caller to call the validator a second
On 2012/04/10 19:04:09, Mark Seaborn wrote:
> 'responsibility'

Done.

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
File src/trusted/validator/x86/32/ncvalidate.c (right):

https://chromiumcodereview.appspot.com/10024039/diff/19/src/trusted/validator...
src/trusted/validator/x86/32/ncvalidate.c:111: guest_addr, data, size,
bundle_size,
On 2012/04/10 19:04:09, Mark Seaborn wrote:
> Nit: indent -2

Done.