Fix some errors reported by Coverity (#6797)
* coverity_20190905: Fix reported bugs This commit has fixes for some bugs reported by Coverity in the present day * coverity_20190905: Fix missing report FIx a missing report of error * coverity_20190905: Pipe close The previous fix had an error that wolud allow a socket continue opened, this commit fixes this * coverity_20190905: Error pattern The call of perror would generate a different error report, instead I am using strerror() to keep pattern * coverity_20190905: Error function Rewrite the call to error function * coverity_20190905: Fix missing tests The previous fix did not have correct tests after to clean the variables * coverity_20190905: Fix readable I changed for an else instead a new if, it is more clean this way * coverity_20190905: remove unecessary test This commit is removing an unecessary test for a variable that will never be NULL. * coverity_20190905: Add neccessary NULLL After to clean the variable, I am setting NULL to variable to avoid clean again * coverity_20190905: Remove false error The condition added to fix Coverity was generating false positives, so we are changing to debug * coverity_20190905: Remove false error The condition added to fix Coverity was generating false positives, so we are changing to debug * coverity_20190905: Bring else to avoid error Bring an else to solve the problem to read a FD not opened * coverity_20190905: Return After to analyse the last changes, I decided to return, because they were not necessary * coverity_20190905: Remove NULL Remove unecessary set of variable to NULL
thiagoftsm committed
Sep 20, 2019 at 11:45 UTC
f555472a8ceb189407b103570be9a28031819d4b
5 files changed
+79
-86
collectors/proc.plugin/proc_mdstat.c
+1
-1
@@ -197,7 +197,7 @@ int do_proc_mdstat(int update_every, usec_t dt) {
197
}
198
s++;
199
}
200
- if(unlikely(str_total[0] == '\0' || str_inuse[0] == '\0')) {
200
+ if(unlikely(str_total[0] == '\0' || !str_inuse || str_inuse[0] == '\0')) {
201
error("Cannot read /proc/mdstat raid health status. Unexpected format.");
202
continue;
203
}
collectors/proc.plugin/sys_class_power_supply.c
+53
-45
@@ -245,67 +245,75 @@ int do_sys_class_power_supply(int update_every, usec_t dt) {
245
if(unlikely(ps->capacity->fd == -1)) {
246
error("Cannot open file '%s'", ps->capacity->filename);
247
power_supply_free(ps);
248
+ ps = NULL;
249
}
250
}
251
251
- ssize_t r = read(ps->capacity->fd, buffer, 30);
252
- if(unlikely(r < 1)) {
253
- error("Cannot read file '%s'", ps->capacity->filename);
254
- power_supply_free(ps);
255
- }
256
- else {
257
- buffer[r] = '\0';
258
- ps->capacity->value = str2ull(buffer);
259
- }
252
+ if (ps)
253
+ {
254
+ ssize_t r = read(ps->capacity->fd, buffer, 30);
255
+ if(unlikely(r < 1)) {
256
+ error("Cannot read file '%s'", ps->capacity->filename);
257
+ power_supply_free(ps);
258
+ ps = NULL;
259
+ }
260
+ else {
261
+ buffer[r] = '\0';
262
+ ps->capacity->value = str2ull(buffer);
263
261
- if(unlikely(!keep_fds_open)) {
262
- close(ps->capacity->fd);
263
- ps->capacity->fd = -1;
264
- }
265
- else if(unlikely(lseek(ps->capacity->fd, 0, SEEK_SET) == -1)) {
266
- error("Cannot seek in file '%s'", ps->capacity->filename);
267
- close(ps->capacity->fd);
268
- ps->capacity->fd = -1;
264
+ if(unlikely(!keep_fds_open)) {
265
+ close(ps->capacity->fd);
266
+ ps->capacity->fd = -1;
267
+ }
268
+ else if(unlikely(lseek(ps->capacity->fd, 0, SEEK_SET) == -1)) {
269
+ error("Cannot seek in file '%s'", ps->capacity->filename);
270
+ close(ps->capacity->fd);
271
+ ps->capacity->fd = -1;
272
+ }
273
+ }
274
}
275
}
276
277
// read property files
278
int read_error = 0;
279
struct ps_property *pr;
275
- for(pr = ps->property_root; pr && !read_error; pr = pr->next) {
276
- struct ps_property_dim *pd;
277
- for(pd = pr->property_dim_root; pd; pd = pd->next) {
278
- if(likely(!pd->always_zero)) {
279
- char buffer[30 + 1];
280
-
281
- if(unlikely(pd->fd == -1)) {
282
- pd->fd = open(pd->filename, O_RDONLY, 0666);
280
+ if (ps)
281
+ {
282
+ for(pr = ps->property_root; pr && !read_error; pr = pr->next) {
283
+ struct ps_property_dim *pd;
284
+ for(pd = pr->property_dim_root; pd; pd = pd->next) {
285
+ if(likely(!pd->always_zero)) {
286
+ char buffer[30 + 1];
287
+
288
if(unlikely(pd->fd == -1)) {
284
- error("Cannot open file '%s'", pd->filename);
289
+ pd->fd = open(pd->filename, O_RDONLY, 0666);
290
+ if(unlikely(pd->fd == -1)) {
291
+ error("Cannot open file '%s'", pd->filename);
292
+ read_error = 1;
293
+ power_supply_free(ps);
294
+ break;
295
+ }
296
+ }
297
+
298
+ ssize_t r = read(pd->fd, buffer, 30);
299
+ if(unlikely(r < 1)) {
300
+ error("Cannot read file '%s'", pd->filename);
301
read_error = 1;
302
power_supply_free(ps);
303
break;
304
}
289
- }
290
-
291
- ssize_t r = read(pd->fd, buffer, 30);
292
- if(unlikely(r < 1)) {
293
- error("Cannot read file '%s'", pd->filename);
294
- read_error = 1;
295
- power_supply_free(ps);
296
- break;
297
- }
298
- buffer[r] = '\0';
299
- pd->value = str2ull(buffer);
305
+ buffer[r] = '\0';
306
+ pd->value = str2ull(buffer);
307
301
- if(unlikely(!keep_fds_open)) {
302
- close(pd->fd);
303
- pd->fd = -1;
304
- }
305
- else if(unlikely(lseek(pd->fd, 0, SEEK_SET) == -1)) {
306
- error("Cannot seek in file '%s'", pd->filename);
307
- close(pd->fd);
308
- pd->fd = -1;
308
+ if(unlikely(!keep_fds_open)) {
309
+ close(pd->fd);
310
+ pd->fd = -1;
311
+ }
312
+ else if(unlikely(lseek(pd->fd, 0, SEEK_SET) == -1)) {
313
+ error("Cannot seek in file '%s'", pd->filename);
314
+ close(pd->fd);
315
+ pd->fd = -1;
316
+ }
317
}
318
}
319
}
health/health.c
+18
-20
@@ -45,32 +45,30 @@ inline char *health_stock_config_dir(void) {
45
* Function used to initialize the silencer structure.
46
*/
47
void health_silencers_init(void) {
48
- struct stat statbuf;
49
- if (!stat(silencers_filename,&statbuf)) {
50
- off_t length = statbuf.st_size;
48
+ FILE *fd = fopen(silencers_filename, "r");
49
+ if (fd) {
50
+ fseek(fd, 0 , SEEK_END);
51
+ off_t length = (off_t) ftell(fd);
52
+ fseek(fd, 0 , SEEK_SET);
53
+
54
if (length && length < HEALTH_SILENCERS_MAX_FILE_LEN) {
52
- FILE *fd = fopen(silencers_filename, "r");
53
- if (fd) {
54
- char *str = mallocz((length+1)* sizeof(char));
55
- if(str) {
56
- size_t copied;
57
- copied = fread(str, sizeof(char), length, fd);
58
- if (copied == (length* sizeof(char))) {
59
- str[length] = 0x00;
60
- json_parse(str, NULL, health_silencers_json_read_callback);
61
- info("Parsed health silencers file %s", silencers_filename);
62
- } else {
63
- error("Cannot read the data from health silencers file %s", silencers_filename);
64
- }
65
- freez(str);
55
+ char *str = mallocz((length+1)* sizeof(char));
56
+ if(str) {
57
+ size_t copied;
58
+ copied = fread(str, sizeof(char), length, fd);
59
+ if (copied == (length* sizeof(char))) {
60
+ str[length] = 0x00;
61
+ json_parse(str, NULL, health_silencers_json_read_callback);
62
+ info("Parsed health silencers file %s", silencers_filename);
63
+ } else {
64
+ error("Cannot read the data from health silencers file %s", silencers_filename);
65
}
67
- fclose(fd);
68
- } else {
69
- error("Cannot open the file %s",silencers_filename);
66
+ freez(str);
67
}
68
} else {
69
error("Health silencers file %s has the size %ld that is out of range[ 1 , %d ]. Aborting read.", silencers_filename, length, HEALTH_SILENCERS_MAX_FILE_LEN);
70
}
71
+ fclose(fd);
72
} else {
73
error("Cannot open the file %s",silencers_filename);
74
}
libnetdata/popen/popen.c
+7
-3
@@ -68,7 +68,7 @@ static inline FILE *custom_popene(const char *command, volatile pid_t *pidptr, c
68
int i;
69
for(i = (int) (sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--)
70
if(i != STDIN_FILENO && i != STDERR_FILENO)
71
- fcntl(i, F_SETFD, FD_CLOEXEC);
71
+ (void)fcntl(i, F_SETFD, FD_CLOEXEC);
72
73
if (!posix_spawn_file_actions_init(&fa)) {
74
// move the pipe to stdout in the child
@@ -97,7 +97,7 @@ static inline FILE *custom_popene(const char *command, volatile pid_t *pidptr, c
97
debug(D_CHILDS, "Spawned command: '%s' on pid %d from parent pid %d.", command, pid, getpid());
98
} else {
99
error("Failed to spawn command: '%s' from parent pid %d.", command, getpid());
100
- close(pipefd[PIPE_READ]);
100
+ fclose(fp);
101
fp = NULL;
102
}
103
close(pipefd[PIPE_WRITE]);
@@ -116,7 +116,11 @@ error_after_posix_spawn_file_actions_init:
116
if (posix_spawn_file_actions_destroy(&fa))
117
error("posix_spawn_file_actions_destroy");
118
error_after_pipe:
119
- close(pipefd[PIPE_READ]);
119
+ if (fp)
120
+ fclose(fp);
121
+ else
122
+ close(pipefd[PIPE_READ]);
123
+
124
close(pipefd[PIPE_WRITE]);
125
return NULL;
126
}
libnetdata/url/url.c
-17
@@ -43,23 +43,6 @@ char *url_encode(char *str) {
43
return pbuf;
44
}
45
46
-/**
47
- * URL Decode
48
- *
49
- * Returns a url-decoded version of str
50
- * IMPORTANT: be sure to free() the returned string after use
51
- *
52
- * @param str the string that will be decode
53
- *
54
- * @return a pointer for the url decoded.
55
- */
56
-char *url_decode(char *str) {
57
- size_t size = strlen(str) + 1;
58
-
59
- char *buf = mallocz(size);
60
- return url_decode_r(buf, str, size);
61
-}
62
-
46
/**
47
* Percentage escape decode
48
*