On 08/29/2017 05:09 PM, Phil Sutter wrote:
The signedness of char type is implementation dependent, and there are
architectures on which it is unsigned by default. In that case, the
check whether fgetc() returned EOF failed because the return value was
assigned an (unsigned) char variable prior to comparison with EOF (which
is defined to -1). Fix this by using int as type for 'c' variable, which
also matches the declaration of fgetc().
While being at it, fix the parser logic to correctly handle multiple
empty lines and consecutive whitespace and tab characters to further
improve the parser's robustness. Note that this will still detect double
separator characters, so doesn't soften up the parser too much.
Fixes: 3da3ebfca85b8 ("bpf: Make bytecode-file reading a little more robust")
Cc: Daniel Borkmann <dan...@iogearbox.net>
Signed-off-by: Phil Sutter <p...@nwl.cc>
Definitely ack on the EOF bug:
Acked-by: Daniel Borkmann <dan...@iogearbox.net>
[...]
@@ -228,18 +229,20 @@ static int bpf_parse_string(char *arg, bool from_file,
__u16 *bpf_len,
case '\n':
if (c_prev != ',')
*(pos++) = ',';
+ c_prev = ',';
break;
case ' ':
case '\t':
if (c_prev != ' ')
*(pos++) = c;
+ c_prev = ' ';
break;
default:
*(pos++) = c;
+ c_prev = c;
}
if (pos - tmp_string == tmp_len)
break;
- c_prev = c;
I don't really have a strong opinion on this, but the logic for
normalizing here is getting a bit convoluted. Is your use case
for making the parser more robust mainly so you can just use the
-ddd output from tcpdump for cBPF w/o piping through tr? But even
that shouldn't give multiple empty lines afaik, no?
