On 10/1/18 3:46 PM, Christophe Lyon wrote: > On Sat, 29 Sep 2018 at 18:06, Jeff Law <l...@redhat.com> wrote: >> >> >> This patch changes the NONSTR argument to c_strlen to instead be a >> little data structure c_strlen can populate with nuggets of information >> about the string. >> >> There's clearly a need for the decl related to the non-string argument. >> I see an immediate need for the length of a non-terminated string >> (c_strlen returns NULL for non-terminated strings). I also see a need >> for the offset within the non-terminated strong as well. >> >> We only populate the structure when c_strlen encounters a non-terminated >> string. One could argue we should always fill in the members. Right >> now I think filling it in for unterminated cases makes the most sense, >> but I could be convinced otherwise. >> >> I won't be surprised if subsequent warnings from Martin need additional >> information about the string. The idea here is we can add more elements >> to the structure without continually adding arguments to c_strlen. >> >> Bootstrapped in isolation as well as with Martin's patches for strnlen >> and sprintf checking. Installing on the trunk. >> > > Hi Jeff, > > + /* If TYPE is asking for a maximum, then use any > + length (including the length of an unterminated > + string) for VAL. */ > + if (type == 2) > + val = data.len; > > It seems this part is dead-code, since the case type==2 is handled in > the "then" part of the "if" (this code is in the "else" part). > > Since you added a comment, I suspect you explicitly tested it, though? Yea, I know that code got triggered at some point. It may be dead now after some cleanups, or it might have been needed by the patch I just installed on the sprintf bits. I'll double-check either way. (I'd seen it in the coverity scans this morning as well).
jeff
