PR#204: Fix error handling in gp_config_from_dir - gssproxy

gssproxy

#204 Fix error handling in gp_config_from_dir

Merged 6 years ago by cipherboy. Opened 6 years ago by cipherboy.

cipherboy/gssproxy error_handling into master

Fix error handling in gp_config_from_dir

Alexander Scheel • 6 years ago

aa680ad

proxy/src/gp_config.c

file modified

+4 -4

		`@@ -823,14 +823,14 @@`
		`if (ret) {`
		`if (error_list) {`
		`uint32_t i;`
		`- uint32_t len = ref_array_getlen(error_list, &i);`
		`+ uint32_t len = ref_array_len(error_list);`
		`for (i = 0; i < len; i++) {`
		`- GPDEBUG("Error when reading config directory: %s\n",`
		`- (const char *) ref_array_get(error_list, i, NULL));`
		`+ GPAUDIT("Error when reading config directory: %s\n",`
		`+ (char *)ref_array_get(error_list, i, NULL));`
		`}`
		`ref_array_destroy(error_list);`
		`} else {`
		`- GPDEBUG("Error when reading config directory number: %d\n", ret);`
		`+ GPAUDIT("Error when reading config directory number: %d\n", ret);`
		`}`
		`return ret;`
		`}`

cipherboy commented 6 years ago

This is a bugfix when handling errors in gp_config_from_dir. Per docs, ref_array_getlen returns a status code (0/EINVAL) and not the length of the array; the second parameter, i, is assigned the length of the array. Thus i >= 0 and len == 0, so i >= len and the for loop is never triggered.

Second, due to the implementation, error_list is a list of pointers to error message strings. This does not need to be freed as ini_config_augment handles this for us.

cipherboy commented 6 years ago

Would you accept GPDEBUG() changed to GPERROR() or fprintf(stderr, ...) as well? I feel that GPERROR() is more fitting in this case, as GPDEBUG() will not work when run as a daemon due to gp_debug not being set. If an error happens while loading the config file, gp_debug_toggle() will not have been called and with the current implementation of gp_debug_args(), gp_debug will be at its default value of zero.