Re: [PATCH 33/33] util: virerror: Remove virReportOOMError

Trying to report an OOM error is pointless since our infrastructure to report error needs to allocate memory to report the error. In addition our code mistakenly reported OOM errors even in cases where a function could fail for another reason, which would make issues harder to debug. Remove the virReportOOMError and backend so that programmers are forced to think about what can happen. In case when there's another failure possible a specific error should be reported and otherwise a direct abort() is better since the logger would abort on g_new anyways. This patch also removes the syntas-check which forces use of virReportOOMError instead of using VIR_ERR_NO_MEMORY with other functions. This allows possible future use when we'd end up in a situation where trying to recover from an OOM would make sense, such as when attempting to allocate a massive buffer. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt;

--- build-aux/syntax-check.mk | 8 -------- src/libvirt_private.syms | 1 - src/util/virerror.c | 22 ---------------------- src/util/virerror.h | 8 -------- 4 files changed, 39 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index e51877648a..e1ccb74986 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -490,11 +490,6 @@ sc_prohibit_gettext_noop: halt='use N_, not gettext_noop' \ $(_sc_search_regexp) -sc_prohibit_VIR_ERR_NO_MEMORY: - @prohibit='\<VIR_ERR_NO_MEMORY\>' \ - halt='use virReportOOMError, not VIR_ERR_NO_MEMORY' \ - $(_sc_search_regexp) - sc_prohibit_PATH_MAX: @prohibit='\<PATH_MAX\>' \ halt='dynamically allocate paths, do not use PATH_MAX' \ @@ -1895,9 +1890,6 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \ exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$) -exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \ - ^(build-aux/syntax-check\.mk|include/libvirt/virterror\.h|src/remote/remote_daemon_dispatch\.c|src/util/virerror\.c|docs/internals/oomtesting\.html\.in)$$ - exclude_file_name_regexp--sc_prohibit_PATH_MAX = \ ^build-aux/syntax-check\.mk$$ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 48f66daab8..7eb37ed797 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2116,7 +2116,6 @@ virLastErrorPrefixMessage; virRaiseErrorFull; virRaiseErrorObject; virReportErrorHelper; -virReportOOMErrorFull; virReportSystemErrorFull; virSetError; virSetErrorLogPriorityFunc; diff --git a/src/util/virerror.c b/src/util/virerror.c index 708081414a..a503cdefdc 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1358,28 +1358,6 @@ void virReportSystemErrorFull(int domcode, errno = save_errno; } -/** - * virReportOOMErrorFull: - * @domcode: the virErrorDomain indicating where it's coming from - * @filename: filename where error was raised - * @funcname: function name where error was raised - * @linenr: line number where error was raised - * - * Convenience internal routine called when an out of memory error is - * detected - */ -void virReportOOMErrorFull(int domcode, - const char *filename, - const char *funcname, - size_t linenr) -{ - const char *virerr; - - virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL); - virRaiseErrorFull(filename, funcname, linenr, - domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR, - virerr, NULL, NULL, -1, -1, virerr, NULL); -} /** * virSetErrorLogPriorityFunc: diff --git a/src/util/virerror.h b/src/util/virerror.h index 9d3e40d65a..da7d7c0afe 100644 --- a/src/util/virerror.h +++ b/src/util/virerror.h @@ -174,14 +174,6 @@ void virReportSystemErrorFull(int domcode, "Unexpected enum value %d for %s", \ value, sizeof((typname)1) != 0 ? #typname : #typname); -void virReportOOMErrorFull(int domcode, - const char *filename, - const char *funcname, - size_t linenr); - -#define virReportOOMError() \ - virReportOOMErrorFull(VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) - #define virReportError(code, ...) \ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__)