Message ID | 54819F98.5080102@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Fri, Dec 05, 2014 at 10:05:44AM -0200, Adhemerval Zanella wrote: > The 'data_len' is calculated with wsclen and allocated as 'char'. The subsequent fgetws > will then try to write at most 'data_len' wchar_t in a buffer with just data_len 'char'. > The following patch fix the issues I am seeing on powerpc64: > > diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c > index f69e169..44a4fac 100644 > --- a/libio/tst-ftell-active-handler.c > +++ b/libio/tst-ftell-active-handler.c > @@ -84,6 +84,7 @@ static const char *char_data = "abcdef"; > static const wchar_t *wide_data = L"abcdef"; > static size_t data_len; > static size_t file_len; > +static size_t char_len; > > typedef int (*fputs_func_t) (const void *data, FILE *fp); > typedef void *(*fgets_func_t) (void *ws, int n, FILE *fp); > @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) > reading. */ > if (test_modes[i].fd_mode != O_WRONLY) > { > - char tmpbuf[data_len]; > + char tmpbuf[data_len * char_len]; > > rewind (fp); > > - while (fgets_func (tmpbuf, sizeof (tmpbuf), fp) && !feof (fp)); > + while (fgets_func (tmpbuf, data_len, fp) && !feof (fp)); > > write_ret = write (fd, data, data_len); > if (write_ret != data_len) > @@ -656,6 +657,7 @@ do_test (void) > fgets_func = (fgets_func_t) fgets; > data = char_data; > data_len = strlen (char_data); > + char_len = sizeof (char); > ret |= do_one_test (filename); > > /* Truncate the file before repeating the tests in wide mode. */ > @@ -678,6 +680,7 @@ do_test (void) > fgets_func = (fgets_func_t) fgetws; > data = wide_data; > data_len = wcslen (wide_data); > + char_len = sizeof (wchar_t); > ret |= do_one_test (filename); > > return ret; > > > What do you think? > Ah yes, thanks for debugging this. Looks good to me. Siddhesh
On 05-12-2014 10:31, Siddhesh Poyarekar wrote: > On Fri, Dec 05, 2014 at 10:05:44AM -0200, Adhemerval Zanella wrote: >> The 'data_len' is calculated with wsclen and allocated as 'char'. The subsequent fgetws >> will then try to write at most 'data_len' wchar_t in a buffer with just data_len 'char'. >> The following patch fix the issues I am seeing on powerpc64: >> >> diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c >> index f69e169..44a4fac 100644 >> --- a/libio/tst-ftell-active-handler.c >> +++ b/libio/tst-ftell-active-handler.c >> @@ -84,6 +84,7 @@ static const char *char_data = "abcdef"; >> static const wchar_t *wide_data = L"abcdef"; >> static size_t data_len; >> static size_t file_len; >> +static size_t char_len; >> >> typedef int (*fputs_func_t) (const void *data, FILE *fp); >> typedef void *(*fgets_func_t) (void *ws, int n, FILE *fp); >> @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) >> reading. */ >> if (test_modes[i].fd_mode != O_WRONLY) >> { >> - char tmpbuf[data_len]; >> + char tmpbuf[data_len * char_len]; >> >> rewind (fp); >> >> - while (fgets_func (tmpbuf, sizeof (tmpbuf), fp) && !feof (fp)); >> + while (fgets_func (tmpbuf, data_len, fp) && !feof (fp)); >> >> write_ret = write (fd, data, data_len); >> if (write_ret != data_len) >> @@ -656,6 +657,7 @@ do_test (void) >> fgets_func = (fgets_func_t) fgets; >> data = char_data; >> data_len = strlen (char_data); >> + char_len = sizeof (char); >> ret |= do_one_test (filename); >> >> /* Truncate the file before repeating the tests in wide mode. */ >> @@ -678,6 +680,7 @@ do_test (void) >> fgets_func = (fgets_func_t) fgetws; >> data = wide_data; >> data_len = wcslen (wide_data); >> + char_len = sizeof (wchar_t); >> ret |= do_one_test (filename); >> >> return ret; >> >> >> What do you think? >> > Ah yes, thanks for debugging this. Looks good to me. > > Siddhesh Pushed.
Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: > @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) > reading. */ > if (test_modes[i].fd_mode != O_WRONLY) > { > - char tmpbuf[data_len]; > + char tmpbuf[data_len * char_len]; tmpbuf may not be aligned for wchar_t. Andreas.
On 05-12-2014 14:23, Andreas Schwab wrote: > Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: > >> @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) >> reading. */ >> if (test_modes[i].fd_mode != O_WRONLY) >> { >> - char tmpbuf[data_len]; >> + char tmpbuf[data_len * char_len]; > tmpbuf may not be aligned for wchar_t. But I can't see why this won't allocate enough space for further operations. Care to explain? > > Andreas. >
Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: > On 05-12-2014 14:23, Andreas Schwab wrote: >> Adhemerval Zanella <azanella@linux.vnet.ibm.com> writes: >> >>> @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) >>> reading. */ >>> if (test_modes[i].fd_mode != O_WRONLY) >>> { >>> - char tmpbuf[data_len]; >>> + char tmpbuf[data_len * char_len]; >> tmpbuf may not be aligned for wchar_t. > > But I can't see why this won't allocate enough space for further operations. Where did I say that? > Care to explain? Alignment and size are separate properties of an object. Andreas.
diff --git a/libio/tst-ftell-active-handler.c b/libio/tst-ftell-active-handler.c index f69e169..44a4fac 100644 --- a/libio/tst-ftell-active-handler.c +++ b/libio/tst-ftell-active-handler.c @@ -84,6 +84,7 @@ static const char *char_data = "abcdef"; static const wchar_t *wide_data = L"abcdef"; static size_t data_len; static size_t file_len; +static size_t char_len; typedef int (*fputs_func_t) (const void *data, FILE *fp); typedef void *(*fgets_func_t) (void *ws, int n, FILE *fp); @@ -364,11 +365,11 @@ do_ftell_test (const char *filename) reading. */ if (test_modes[i].fd_mode != O_WRONLY) { - char tmpbuf[data_len]; + char tmpbuf[data_len * char_len]; rewind (fp); - while (fgets_func (tmpbuf, sizeof (tmpbuf), fp) && !feof (fp)); + while (fgets_func (tmpbuf, data_len, fp) && !feof (fp)); write_ret = write (fd, data, data_len); if (write_ret != data_len) @@ -656,6 +657,7 @@ do_test (void) fgets_func = (fgets_func_t) fgets; data = char_data; data_len = strlen (char_data); + char_len = sizeof (char); ret |= do_one_test (filename); /* Truncate the file before repeating the tests in wide mode. */ @@ -678,6 +680,7 @@ do_test (void) fgets_func = (fgets_func_t) fgetws; data = wide_data; data_len = wcslen (wide_data); + char_len = sizeof (wchar_t); ret |= do_one_test (filename); return ret;