Fix storage_lookup_patches return code in case process already termin… #35
Conversation
rashchupkinr
force-pushed
the
master
branch
2 times, most recently
from
6e57238
to
d90314e
Compare
If process terminated before storage_lookup_patches, then no buildids can be found and function must return -1.
|
|
||
| static void __valog(int level, const char *prefix, const char *fmt, va_list va) | ||
| { | ||
| FILE *f = level <= LOG_WARN ? stderr : stdout; | ||
| if (prefix) | ||
| fprintf(f, "%s", prefix); | ||
|
|
||
| if (log_file) { | ||
| va_list vaf; |
There was a problem hiding this comment.
please print prefix as well.
src/kpatch_log.c
Outdated
| @@ -8,13 +8,20 @@ | |||
|
|
|||
| int log_level = LOG_INFO; | |||
| int log_indent; | |||
| FILE *log_file; | |||
There was a problem hiding this comment.
please either init this to zero or better make it static.
src/kpatch_user.c
Outdated
| @@ -566,6 +566,10 @@ cmd_update(int argc, char *argv[]) | |||
|
|
|||
| #ifdef STRESS_TEST | |||
|
|
|||
| #define LOG_NAME_LEN 1024 | |||
src/kpatch_user.c
Outdated
| @@ -566,6 +566,10 @@ cmd_update(int argc, char *argv[]) | |||
|
|
|||
| #ifdef STRESS_TEST | |||
|
|
|||
| #define LOG_NAME_LEN 1024 | |||
| char test_log_base[LOG_NAME_LEN]; | |||
There was a problem hiding this comment.
please init these to a empty string (or declare them as static).
src/kpatch_user.c
Outdated
| return -1; | ||
| } | ||
| if (log_file_init(test_log_name)) { | ||
| kperr("Can't open log file \'%s\'\n", test_log_name); |
There was a problem hiding this comment.
please use kplogerror here.
src/kpatch_user.c
Outdated
| kperr("Can't parse pid from %s\n", argv[1]); | ||
| return -1; | ||
| } else | ||
| kpinfo("Fork for pid=%d\n", pid); |
There was a problem hiding this comment.
please write a better log message. Like 'spawning child to patch pid %d'.
src/kpatch_user.c
Outdated
| int child = fork(); | ||
| if (child == 0) { | ||
| int rv = server_stress_test(fd, argc, argv); | ||
| stress_test_log_init(pid); |
There was a problem hiding this comment.
This returns value. Check it.
There was a problem hiding this comment.
Error messages are written inside stress_test_log_init.
Contrary to base process, children should continue work even if can't initialize log file.
Is it better for them to exit in case of logging error too?
src/kpatch_user.c
Outdated
| kpfatal("Can't initialize log\n"); | ||
| break; | ||
| } | ||
| strncpy(test_log_base, optarg, LOG_NAME_LEN); |
There was a problem hiding this comment.
Is it possible to just use strncpy and then check if last byte is zero? (this requires test_log_base to be declared static).
rashchupkinr
force-pushed
the
master
branch
14 times, most recently
from
a6b9e9d
to
fb0f8af
Compare
-change some printfs to logging
rashchupkinr
force-pushed
the
master
branch
4 times, most recently
from
801f8eb
to
786a494
Compare
Sometimes during stress tests buildids can't be retrieved because process already terminated.
In this case "No patch(es) applicable to PID '%d' have been found\n" was printed, which is considered error by test scripts.