Hi Jerry,
On 9/22/19 10:51 PM, Jerry DeLisle wrote:
The attached patch eliminates several warnings by adjusting which
enumerator is used in the subject offending code. I fixed this by
adding an enumerator at the end of the file_mode definition. This
then triggered a warning in several other places for an unhandled case
in the switch statements. I cleared those by throwing in an assert
(false) since it cant happen unless something really goes wrong somehow.
Regardless, regression tested on x86_64-pc-linux-gnu.
OK for trunk? No applicable test case.
LGTM – thanks for eliminating warnings.
PS
While I was at it, I 'touched' all files in libgfortran/io to see what
other warnings are left, There is another odd warning I have not
sorted out yet.
[…]
In function ‘btoa_big’,
inlined from ‘write_b’ at
../../../trunk/libgfortran/io/write.c:1212:11:
../../../trunk/libgfortran/io/write.c:1051:6: warning: writing 1 byte
into a region of size 0 [-Wstringop-overflow=]
1051 | *q = '\0';
| ~~~^~~~~~
The first of these two I understand. The second one about region of
size 0 puzzles me.
btoa_big (const char *s, char *buffer, int len, GFC_UINTEGER_LARGEST *n)
…
q = buffer;
…
for (i = 0; i < len; i++)
for (j = 0; j < 8; j++)
{
*q++ = (c & 128) ? '1' : '0';
…
*q = '\0';
I think the compiler assumes that if you call "q++" (alias buffer)
"8*len" times, that the
*q = '\0';
will write at buffer[len] – which could be one byte beyond the buffer
size. I don't quickly see whether that's the case or not, but I think
you should check whether this can happen – and if not, you may need to
add a comment, e.g. stating that the buffer is 8*len+1 bytes long or
something along that line. And if it can happen, you need to ensure that
in the future it cannot :-)
Now looking at the code,
char itoa_buf[GFC_BTOA_BUF_SIZE];
with
#define GFC_BTOA_BUF_SIZE (GFC_LARGEST_BUF * 8 + 1)
and GFC_LARGEST_BUF is sizeof(real-16 real or integer-16) if available
or sizeof(real-10) or is not sizeof(largest integer).
"len" comes in as parameter to write_b/write_o/write_z and looks like
being the byte size or kind. Hence, it seems to be fine. Maybe adding a
comment plus an assert(len < GFC_BTOA_BUF_SIZE) would make sense? With
the assert, the warning would return with "NDEBUG" set (cf. assert man
page) but otherwise, it should be fine.
Down side is that w/o NDEBUG, one adds one additional condition ("if"
branch) to the code - even if it is regarded as unlikely (assert code
moved to the end of the function) - and adds some strings + printf call
as well (via the assert). But at the end, one probably doesn't need to
worry about this too much.
Cheers,
Tobias
