LLNL / UnifyFS

UnifyFS: A file system for burst buffers
Other
107 stars 31 forks source link

Size mismatch in write() call (in test code) #782

Closed rgmiller closed 1 year ago

rgmiller commented 1 year ago

System information

Type Version/Name
Operating System Ubuntu
OS Version 22.04
Architecture x86_64
UnifyFS Version latest from git

Describe the problem you're observing

https://github.com/LLNL/UnifyFS/blob/f95abed7c1aa91864d047b7c0ad38ac05e5719a7/t/sys/write-read.c#L379

The call to write() on this line specifies a count of 300, but the data to be written is only 2 bytes long. Newer versions of gcc (at least 11.3.0, possibly somewhat earlier) will issue a warning about this. And by default, we build with -Werror enabled, so the warning turns into an error and the build stops.

Temporary work-around is to compile with -Wno-error=stringop-overread.

Describe how to reproduce the problem

Just look at the code.

Include any warning or errors or releveant debugging data

The exact error from GCC:

./../t/sys/write-read.c: In function ‘write_pre_existing_file_test’:
../../t/sys/write-read.c:379:16: error: ‘write’ reading 300 bytes from a region of size 2 [-Werror=stringop-overread]
  379 |     rc = (int) write(fd, "a", 300);
      |                ^~~~~~~~~~~~~~~~~~~
adammoody commented 1 year ago

Good catch. I think maybe that buf was intended for this parameter instead of "a"?

https://github.com/LLNL/UnifyFS/blob/f95abed7c1aa91864d047b7c0ad38ac05e5719a7/t/sys/write-read.c#L364

rgmiller commented 1 year ago

That buffer is initialized to all zeros. Is that sufficient for this test? Or should we have different values in that buffer?

Also, the code uses the literal 300 in a couple of different places. Should we replace that with a #define or const int?

MichaelBrim commented 1 year ago

@rgmiller Based on what it's testing, I don't think the zeroes are a problem. I agree that we should use a constant or #define for the buffer size.