This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Add a character size parameter to c_strlen/get_range_strlen
On 08/15/18 07:12, Jeff Law wrote:
> On 08/14/2018 08:08 AM, Martin Sebor wrote:
>> On 08/14/2018 04:25 AM, Bernd Edlinger wrote:
>>> Hi,
>>>
>>> this patch is a follow-up to my patch here:
>>> https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01800.html
>>
>> As I said multiple times, this patch is redundant -- the issue
>> is fixed by the solution I posted back in July:
>>
>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00155.html
>>
>> and that I have been continuing to build upon (I posted a new
>> update yesterday).
> So it seems the question here is whether or not Bernd agrees that it's
> redundant or he may question if your patch is going to go forward since
> it hasn't been acted upon. He didn't say either way which is
> unfortunate. But I don't think it's meant to be malicious.
>
Yes, definitely. What I did was not done to make your life miserable.
But must admit, that I have sometimes the impression of being ignored,
for instance what I wrote in response to Martin's patch here:
https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00184.html
... and already earlier here:
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01878.html
dropped completely on the floor.
> Let's not go back to where we were last week. I think that discussion
> got far too heated.
>
> --
>
>
> Standard procedure in this kind of situation where we have two patches
> that are handling the same issue is to bake them off and choose one
> based on the technical merits. To that end...
>
> It would be helpful if you could compare/contrast your patch to Bernd's.
> ie, are there cases that will be handled by one patch better than the
> other. Are there any implementation details that favor one patch over
> the other.
>
> Similarly it'd be helpful to have Bernd do the same thing for his patch
> compare and contrasting it to yours.
>
I think the other patch fixes a few things but more or less by chance.
At the same time the patch looks to me just *weird*, as if it might introduce
even more bugs.
But I did not find the test cases PR 86711/86714 or the earlier ones just
by chance.
I looked at the functions c_strlen and string_constant, and spotted design issues.
That is these functions were recently extended to handle more cases and return
different values than they did before gcc-7, but not all callers are able to handle
the new functions correctly that they don't need.
All in all, I am pretty sure that there are more hidden bugs in that area.
That's why I feel the need to fix the design and wrong code bugs in the
string handling *before* adding any new warnings.
A new warning does not need to fix a wrong code bug at the same time.
Bernd.