This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #16398] Fix infinite loop in ftell when writing wide char data
- From: Allan McRae <allan at archlinux dot org>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>, Carlos O'Donell <carlos at redhat dot com>
- Cc: Roland McGrath <roland at hack dot frob dot com>, Rich Felker <dalias at aerifal dot cx>, libc-alpha at sourceware dot org
- Date: Tue, 04 Feb 2014 14:29:18 +1000
- Subject: Re: [PATCH][BZ #16398] Fix infinite loop in ftell when writing wide char data
- Authentication-results: sourceware.org; auth=none
- References: <20140131051923 dot GL2149 at spoyarek dot pnq dot redhat dot com> <20140131190337 dot GX24286 at brightrain dot aerifal dot cx> <20140131192607 dot 05D5474430 at topped-with-meat dot com> <20140203095340 dot GE5209 at spoyarek dot pnq dot redhat dot com> <52EFC985 dot 7070408 at redhat dot com> <20140204042150 dot GH5209 at spoyarek dot pnq dot redhat dot com>
On 04/02/14 14:21, Siddhesh Poyarekar wrote:
> On Mon, Feb 03, 2014 at 11:53:25AM -0500, Carlos O'Donell wrote:
>> On 02/03/2014 04:53 AM, Siddhesh Poyarekar wrote:
>>> On Fri, Jan 31, 2014 at 11:26:06AM -0800, Roland McGrath wrote:
>>>> Most test cases are pretty small such that either copyright doesn't apply
>>>> (if really tiny) or it's simple enough to rewrite the case from scratch
>>>> once you roughly grok the bug.
>>>
>>> OK, I just rewrote the test case. Tested on x86_64 and ppc64. OK to
>>> commit?
>>
>> In the future, to be entirely kosher, I think someone other than you should
>> have rewritten the test case given a description of the bug provided by you.
>>
>> I'm hoping that you didn't look too closely at the original test case and
>> that the original copyright owner of said test case doesn't actually care.
>>
>> However, in the future please find someone else to rewrite the test case.
>> If you think I'm being too paranoid please say so, otherwise I'll continue
>> to be at this level of paranoia.
>
> I sent an email to the original author, but did not get any response.
> I did look closely at the original test case though - that's how I
> figured out the bug. However, the test I have written now is
> different from that test:
>
> - I don't have a hard-coded set of strings; I generate a set of just
> two strings from a single character.
>
> - I adapted the test case to plug into our testsuite
>
> - Other than that, the test is a simple fputws, ftell in a loop and
> finally an fclose(), which is barely 10 lines of code.
>
> I don't know what level of paranoia is appropriate for such things
> since I don't have enough experience on it, so I'll just assume that
> it's appropriate. Let me know if my effort to differentiate this test
> from the original one is not enough so that I will just stick to just
> committing the patch and let someone else write the test from scratch.
>
>>> +/* Defined in test-skeleton.c. */
>>> +static int create_temp_file (const char *base, char **filename);
>>> +
>>> +/* Large enough that the target buffer during conversion is not large
>>> + enough. I found this by just tinkering with the numbers till I found a
>>> + small enough number. */
>>
>> What's wrong with making this number larger? Why does it have to be small enough?
>
> Nothing really, just to make it that bit faster. In fact the minimum
> number is somewhere around 1387 or something like that. I could just
> write it up as:
>
> /* Arbitrary number large enough that the target buffer during
> conversion is not large enough. */
>
> since the 'small enough' just adds confusion and nothing else.
>
>>> + /* Generate input from one character. */
>>> + wchar_t seed = L'ã';
>>
>> Please identify the character explicitly in a comment including
>> source language and UTF number and why you use this particular
>> cahracter.
>
> This is again an arbitrary character I chose because it seems to
> occupy an odd number of bytes after conversion, making the condition
> easy to reproduce. I could write this as:
>
> /* Generate input from one character. The character is arbitrarily
> chosen since it seems to occupy an odd number of bytes after
> conversion. */
>
> Or do you want me to find out which character that is? It'll be a
> fair bit of work...
>
NAME Hiragana letter GO
CHAR ã
UTF-8 E38194
UCS 3054
MARC-8 692434