Conversation
this commit fixes the build on MacOS, when using GCC. Building through clang is still not supported. Tested on Apple Silicon Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
|
Hi @alexmohr For this one I am new to it, and need sometimes to verify. |
Signed-off-by: Alexander Mohr <alexander.m.mohr@mercedes-benz.com>
@minminlittleshrimp to make it easier to verify this and to prevent breaking it again in the future, I added a build stage similar to #816. |
| memset(tmp_filename, 0, FILENAME_SIZE); | ||
| #if defined(__GNUC__) && __GNUC__ >= 7 | ||
| # pragma GCC diagnostic push | ||
| # pragma GCC diagnostic ignored "-Wformat-truncation" |
There was a problem hiding this comment.
Instead of ignore, can we perform a proper fix here for truncation instead?
There was a problem hiding this comment.
We could do something like this, so print an error, free files, then exit.
Optionally we could skip freeing, return -1 exits main anyway.
int ret = snprintf(tmp_filename, FILENAME_SIZE, "%s%s",
DLT_CONVERT_WS, files[index - optind + 2]->d_name);
if (ret < 0 || ret >= FILENAME_SIZE) {
fprintf(stderr, "ERROR: Filename too long: %s%s\n",
DLT_CONVERT_WS, files[index - optind + 2]->d_name);
if (ovalue) {
close(ohandle);
ohandle = -1;
}
dlt_file_free(&file, vflag);
if (files) {
for (int i = 0; i < n ; i++)
if (files[i])
free(files[i]);
free(files);
}
return -1;
}There was a problem hiding this comment.
Yes, please rework on the approach, it is fine to truncate properly with snprintf + a ret value to keep compiler satisfied
There was a problem hiding this comment.
it is fine to truncate properly
But that might lead to tmp_filename containing an invalid path. I'm not sure if it's a good idea to truncate and warn because it will lead to invalid files.
As this error is in dlt-convert I think it would be better to do an approach like above and exit with an error instead working with a truncated file name.
But if you prefer truncation I'm cool with that, then I will rework it so file names will be truncated but a warning printed.
Let me know what you think is best :)
There was a problem hiding this comment.
@minminlittleshrimp if you find the time, I would appreciate your input here :)
There was a problem hiding this comment.
Sure, no problem @alexmohr
My team member is working on this topic
| memset(tmp_filename, 0, FILENAME_SIZE); | ||
| #if defined(__GNUC__) && __GNUC__ >= 7 | ||
| # pragma GCC diagnostic push | ||
| # pragma GCC diagnostic ignored "-Wformat-truncation" |
There was a problem hiding this comment.
Yes, please rework on the approach, it is fine to truncate properly with snprintf + a ret value to keep compiler satisfied
|
Team member who take this: we can make use of yaml file from alex to perform checking, we dont need virt setup for MacOS |
this commit fixes the build on MacOS, when using GCC. Building through clang is still not supported.
Tested on Apple Silicon and GCC-15
Alexander Mohr alexander.m.mohr@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH
Provider Information