john icon indicating copy to clipboard operation
john copied to clipboard

Document/review --test-full

Open magnumripper opened this issue 4 years ago • 6 comments

I have no clue what the option does now. There seem to be no documentation other than the usage blob, which seems to be incorrect. It's listed as non-optional parameter but seems to be optional.

See https://github.com/openwall/john/issues/3514#issuecomment-799751758

I guess only @jfoug knows now, or do you @claudioandre-br or @solardiz ?

magnumripper avatar Mar 15 '21 21:03 magnumripper

I have no clue what the option does now.

More tests are performed when/if --test-full is used (within formats.c see var full_lvl). I'm ok with the lack in docs (it is a kind of internal stuff).

Anyway, LEVEL should'nt be optional and john is misbehaving (IMO) when LEVEL is not informed.

ghost avatar Mar 15 '21 22:03 ghost

So the usage blob is correct for showing a required option.

--test-full=LEVEL          run more thorough self-tests

This is strange though: The code specifies it correctly too, with OPT_REQ_PARAM

	{"test-full", FLG_TEST_SET, FLG_TEST_CHK, 0, TEST_REQ_CLR & OPT_REQ_PARAM, "%d", &benchmark_level},

Yet somehow it's not required:

./john -form:nt -test-full

Make process completed.
Benchmarking: NT [MD4 256/256 AVX2 8x3]... DONE
Raw:	103829K c/s real, 103829K c/s virtual

Other options using OPT_REQ_PARAM works as expected:

./john -fork
Option requires a parameter: "-fork"

Edits: Bisected to 051af51ac, simple logic error fixed in d2657fa7f

magnumripper avatar Mar 15 '21 22:03 magnumripper

With that bug fixed: Apparently --test-full=-1 is a dumb way to get --test.

Level 0 activates the extended tests. There are currently no higher level defined, any positive number just behaves the same as 0.

I suggest we make that parameter NOT required and default to 0, so --test-full behaves like --test-full=0. Then at least add a brief description in doc/OPTIONS.

magnumripper avatar Mar 15 '21 23:03 magnumripper

I suggest we make that parameter NOT required and default to 0, so --test-full

John no longer uses the value n in --test-full=n. Agree.

ghost avatar Mar 15 '21 23:03 ghost

I'd prefer --test-full to be the same as --test in how it handles its parameter - that is, make it optional and have it mean seconds for the benchmark. The only expected difference between the options would then be that --test-full would, like the name implies, run full(er) self-tests prior to the (optional) benchmark (skipped with =0).

solardiz avatar Mar 16 '21 12:03 solardiz

I'd prefer --test-full to be the same as --test in how it handles its parameter - that is, make it optional and have it mean seconds for the benchmark.

I dislike the idea. A -test-full = 5 is not intended as a benchmark

We shouldn't forget that -test-full=n is a --test=0,n . In the past john has --test=0,0 and --test=0,1 (more tests).

PS: At this right moment, john has --test=0,0 only.

ghost avatar Mar 16 '21 13:03 ghost