This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [committed] Use structure to bubble up information about unterminated strings from c_strlen


On 10/1/18 3:46 PM, Christophe Lyon wrote:
> On Sat, 29 Sep 2018 at 18:06, Jeff Law <law@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]