Markus Elfring wrote:
Hello,
Some signal handler implementations are also affected by technical details that are described in an article by Justin Pincar. https://www.securecoding.cert.org/confluence/display/seccode/SIG30-C.+Call+o...
Your code looks even worse then your arguments for "correct" codding practices.
-static int watchdog; +static sig_atomic_t watchdog;
Don't do this, you've been told to leave it alone.
- fprintf( stderr, "wineserver crashed, please enable coredumps (ulimit -c unlimited) and restart.\n");
- char const message[] = "wineserver crashed, please enable coredumps (ulimit -c unlimited) and restart.\n";
Should be "static const char" in that exact order.
- (void) write(STDERR_FILENO, message, sizeof(message) - 1);
What's "(void)" doing here? Yes we do ignore returned states here and in 10000s or other places. Don't do that.
- (void) fsync(STDERR_FILENO);
Why do you need sync here?
abort();
--- a/tools/wmc/wmc.c
Wine fix per program please. You'll have to split this into 3 separate patches.
static void segvhandler(int sig) { -<----->fprintf(stderr, "\n%s:%d: Oops, segment violation\n", input_name, line_number); -<----->fflush(stdout); -<----->fflush(stderr);
- char const message[] = "\nOops, segment violation\n";
Same as above - should be "static const char" in that exact order.
- (void) write(STDERR_FILENO, message, sizeof(message) - 1);
- (void) fsync(STDOUT_FILENO);
- (void) fsync(STDERR_FILENO);
Remove bogus "(void)".
<----->abort(); }
Original printed more useful information. Just dropping it in sake of "correctness" is wrong. You'll have to come up with something that prints the same or don't touch it.
Original used tabs (8-spaces) your patch uses 4-space indentation. Either keep the file's formatting or stick to 4-space indentation but DO NOT mix and match both inside one function.
Same applies to tools/wrc/wrc.c
Vitaliy.
-
Vitaliy Margolen