| Message ID | 1532004387-3123-4-git-send-email-thuth@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Clean-ups and fixes for the build_romfs tool | expand |
Hi! On Thu, Jul 19, 2018 at 02:46:26PM +0200, Thomas Huth wrote: > +#define min(a, b) ((a) < (b) ? (a) : (b)) This function-like macro isn't very function-like (it does (potential) double evaluation of its arguments, it can exhibit UB where a function cannot, etc.) Write this as an inline function? Or at the very least give it nature's yellow-and-black-warning stripes, which is ALL CAPS in C ;-) > + pcVersion = getenv("DRIVER_NAME"); > + if (!pcVersion) > + pcVersion = getenv("USER"); > + if (!pcVersion) > + pcVersion = "unknown"; > + memcpy(stHeader.version, pcVersion, > + min(strlen(pcVersion), sizeof(stHeader.version))); You would think there should be a nicer way to do this. I don't see it either. Segher
diff --git a/romfs/tools/create_crc.c b/romfs/tools/create_crc.c index 475b184..b44c911 100644 --- a/romfs/tools/create_crc.c +++ b/romfs/tools/create_crc.c @@ -23,6 +23,8 @@ #include "createcrc.h" #include "crclib.h" +#define min(a, b) ((a) < (b) ? (a) : (b)) + /* file length in bytes */ static uint64_t ui64globalFileSize = 0; /* space for the file stream >= 4MB + 4bytes */ @@ -80,13 +82,13 @@ createHeaderImage(int notime) }; /* read driver info */ - if (NULL != (pcVersion = getenv("DRIVER_NAME"))) { - strncpy(stHeader.version, pcVersion, 16); - } else if (NULL != (pcVersion = getenv("USER"))) { - strncpy(stHeader.version, pcVersion, 16); - } else if (pcVersion == NULL) { - strncpy(stHeader.version, "No known user!", 16); - } + pcVersion = getenv("DRIVER_NAME"); + if (!pcVersion) + pcVersion = getenv("USER"); + if (!pcVersion) + pcVersion = "unknown"; + memcpy(stHeader.version, pcVersion, + min(strlen(pcVersion), sizeof(stHeader.version))); if (!notime) { /* read time and write it into data stream */
GCC 8 complains about the following usages of strncpy, too: create_crc.c:86:3: warning: ‘strncpy’ specified bound 16 equals destination size [-Wstringop-truncation] strncpy(uHeader.stHeader.version, pcVersion, 16); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ create_crc.c:84:3: warning: ‘strncpy’ specified bound 16 equals destination size [-Wstringop-truncation] strncpy(uHeader.stHeader.version, pcVersion, 16); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Let's work around the issue by using memcpy instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- romfs/tools/create_crc.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)