这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ms705
Copy link
Member

@ms705 ms705 commented Aug 10, 2021

Removes many compiler warnings to make it easier to spot important warnings and errors in the output.

  1. Replaces deprecated tmpnam(3) where used (one piece of dead code, one actual implementation in Pipe.cc).
  2. Disables #warning directive output (which gets repeated every time a header is included) by default. Use SHOW_EXPLICIT_WARNINGS to turn it back on. Most of this relates to unimplemented features, so doesn't need to be shown by default.

ms705 added 3 commits August 10, 2021 14:58
Calls tmpnam(3), which is deprecated.
tmpnam(3) is deprecated and triggers compiler warnings.
The #warning directives create a lot of output spam that makes it
hard to see "real" warnings.

Adds an option (SHOW_EXPLICIT_WARNINGS) to enable them (default
off), otherwise suppresses them via -Wno-cpp.
@ms705 ms705 requested a review from LeonhardFS August 10, 2021 21:51
@ms705 ms705 changed the title Reduce compiler warnings [BUILD] Reduce compiler warnings Aug 10, 2021
ofs<<file_input;
ofs.close();
cmd += " " + tmppath + ".py";
char tmpname[22];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should prob. reuse the scratchDir from Tuplex. Per default it's set to /tmp/tuplex-<user>. This will avoid cluttering /tmp with a ton of temp files and has the other advantage that there is a single scratch dir (per user) on which files are stored on. I.e., I would introduce a default parameter scratchDir="/tmp" and then create the file template via
scratchDir + "/pipe-XXXXXX.tmp". (some name without .py because the Pipe class is not python bound).

Also would strongly urge to use snprintf instead of sprintf to avoid buffer overflow/security risk.

Error handling for std::FILE* tmp and fd is further missing (check for NULL/https://man7.org/linux/man-pages/man3/mkstemp.3.html). mkstemps does not always succeed, these scenarios should be covered. Is this function working under Mac?


# suppress "#warning" directive otuput
option(SHOW_EXPLICIT_WARNINGS "Show the output of #warning directives in the code (lots of output)" OFF)
if(SHOW_EXPLICIT_WARNINGS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add message(STATUS "Disabled output of #warning directives in the code, use SHOW_EXPLICIT_WARNINGS to turn all warnings on") or so?

Copy link
Contributor

@LeonhardFS LeonhardFS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should make the replacement code for tempname a bit safer, less prone to errors.

@LeonhardFS LeonhardFS merged commit d048fea into tuplex:master Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants