On 08/17/2018 12:23 PM, Martin Sebor wrote: > On 08/17/2018 06:14 AM, Joseph Myers wrote: >> On Fri, 17 Aug 2018, Jeff Law wrote: >> >>> On 08/16/2018 05:01 PM, Joseph Myers wrote: >>>> On Thu, 16 Aug 2018, Jeff Law wrote: >>>> >>>>> restores previous behavior. The sprintf bits want to count element >>>>> sized chunks, which for wchars is 4 bytes (that count will then be >>>> >>>>> /* Compute the range the argument's length can be in. */ >>>>> - fmtresult slen = get_string_length (arg); >>>>> + int count_by = dir.specifier == 'S' || dir.modifier == FMT_LEN_l >>>>> ? 4 : 1; >>>> >>>> I don't see how a hardcoded 4 is correct here. Surely you need to >>>> example >>>> wchar_type_node to determine its actual size for this target. >>> We did kick this around a little. IIRC Martin didn't think that it was >>> worth handling the 2 byte wchar case. > > Sorry, I think we may have miscommunicated -- I didn't think it > was useful to pass a size of the character type to the function. > I agree that passing in a hardcoded constant doesn't seem right > (even if GCC's wchar_t were always 4 bytes wide). > > I'm still not sure I see the benefit of passing in the expected > element size given that the patch causes c_strlen() fail when > the actual element size doesn't match ELTSIZE. If the caller > asks for the number of bytes in a const wchar_t array it should > get back the number bytes. (I could see it fail if the caller > asked for the number of words in a char array whose size was > not evenly divisible by wordsize.) Interestingly enough that check failing was instrumental in debugging a failing testcase when I was poking a bit at Bernd's patch. I probably would have found the issue without that check, but it was pretty obvious as I stepped through c_strlen.
I don't have a strong opinion on the check (keep vs drop). I do think that passing in the size makes more sense than trying to determine it within c_strlen. The latter is going to stumble over the C vs GIMPLE semantics issues we've discussed elsewhere. Jeff
