-
Notifications
You must be signed in to change notification settings - Fork 317
Description
Lets start with - I found this issue because I was stupid.
Many cmdline tools accept short and long form arguments.
So, when I read the seqtk sample
help, I assumed we could provide either -sXXX
or -seed=XXX
for the seed:
Usage: seqtk sample [-2] [-s seed=11] <in.fa> <frac>|<number>
Options: -s INT RNG seed [11]
-2 2-pass mode: twice as slow but with much reduced memory
When providing -seed=XXX
, seqtk sample
doesn't fail, it silently uses 0
as the seed.
This happens because:
The number of arguments provided passes the getopt()
call here.
The string check passes here.
Then, atol
is passed a null
string and initializes kr_srand()
with a seed of 0.
(I was expecting the default value, 11, but it behaves like -s0
instead)
The same thing happens if we pass -se
as the seed.
(Tested b/c I thought it was parsing e
as its ascii value, 101. This is not the case.)
-seed XXX
does fail, which is nice.
While it would be cool to parse long-form args, I don't think that's worth the effort.
I think clarifying the documentation is more straightforward:
Change this line from [-s seed=11]
to just [-s]
Change this line from RNG seed [11]\n
to RNG seed [default=11]\n
That would make the accepted argument clear while also denoting the default seed.
~Jared