qa_utils token parsing might be insufficient

[michaelld]

This line < https://github.com/n-west/volk/blob/master/lib/qa_utils.cc#L157 >, according to @ValZapod < https://github.com/gnuradio/volk/issues/59#issuecomment-573655898 >:

Is currently:

            if(token[0] == 'x' && (token.size() > 1) && (token[1] > '0' || token[1] < '9')) { //it's a multiplier

Consider instead

            if(token[0] == 'x' && (token.size() > 1) && (token[1] >= '0' && token[1] <= '9')) { //it's a multiplier

[ValZapod]

token[1] > '0' || token[1] < '9' is always true. So may as well just delete it.

[michaelld]

Indeed LOL! Can we have a multiplier that's '0'?

[michaelld]

Does anyone know if we have a place where we describe volk kernel name parsing?

Meaning: Looking at the kernel volk_16i_branch_4_state_8.h, I have 6 tokens (assuming separator _). for volk_32f_x2_dot_prod_16i.h I have 6 tokens as well, but they are clearly somewhat different tokens and the kernels are of course very different. Do we have a file somewhere that describes how to parse these names to make sense of them? I've been poking through the repo & have found nothing obvious just yet ... and it's not like the repo has tons of files in it!

[ValZapod]

I have many such issues, submitting it))

[michaelld]

OK so yes Doug says x[0-9]+ for the multiplier. What I'm going for is: what is a multiplier & can it really be 0? Can it be 15? We need to fix that line anyway, so let's do it correctly for the reality of what the multiplier is. Hence my query about kernel naming ...

[michaelld]

@ValZapod any thoughts on this Volk issue? care to put together a PR for it?

[ValZapod]

@michaelld The code here does nothing (token[1] > '0' || token[1] < '9'). It is always true. I do not know the codebase but why it could be dangerous not to check for 0-9?

[michaelld]

@ValZapod the token should be "x#", where # is a number in [0-9] ... but I'm not 100% sure if we can have "x0" ... certainly "x[2-5]" since those exist. So ... thinking just leaving the check to be in [0-9] is the way to go, just to fix this to be && rather than || as noted in the issue opening.

[ValZapod]

certainly "x[2-5]" since those exist

Perfect. Then just change || to &&. And when (if) you need x0 multiplier edit the code.

[michaelld]

certainly "x[2-5]" since those exist

Perfect. Then just change || to &&. And when (if) you need x0 multiplier edit the code.

yup. I like this tweak ... simple, keeps what I think the intent of the code was supposed to be.