[libvirt] [PATCH] uml: Fix weird logic inside umlConnectOpen() function.
The pointer related to uml_driver needs to be checked before its usage inside the function. Some attributes of the driver are being accessed while the pointer is NULL considering the current logic. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/uml/uml_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index fcd468243e..d1c71d8521 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1193,6 +1193,13 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); + /* URI was good, but driver isn't active */ + if (uml_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("uml state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + /* Check path and tell them correct path if they made a mistake */ if (uml_driver->privileged) { if (STRNEQ(conn->uri->path, "/system") && @@ -1211,13 +1218,6 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, } } - /* URI was good, but driver isn't active */ - if (uml_driver == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("uml state driver is not active")); - return VIR_DRV_OPEN_ERROR; - } - if (virConnectOpenEnsureACL(conn) < 0) return VIR_DRV_OPEN_ERROR; -- 2.17.1
I reworded the commit subject a little and pushed. One more thing to note is that we don't put a 'period' at the end of a commit subject, I've noticed it in a few of your patches, it's just I kept removing them intuitively and always forgot to mention it :). Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 9/28/18 12:13 AM, Julio Faracco wrote:
The pointer related to uml_driver needs to be checked before its usage inside the function. Some attributes of the driver are being accessed while the pointer is NULL considering the current logic.
I see it's pushed already... Perhaps something in the future to consider - when finding stuff like this, look for the commit that broke the original logic, then indicate in the commit message what you found. For this commit 0420a0324 changed to check @uml_driver->privileged before !uml_driver was checked.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/uml/uml_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index fcd468243e..d1c71d8521 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1193,6 +1193,13 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
+ /* URI was good, but driver isn't active */
^^ This comment is wrong now ;-) No big deal... In any case the logic at least is the same as most other drivers now. John
+ if (uml_driver == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("uml state driver is not active")); + return VIR_DRV_OPEN_ERROR; + } + /* Check path and tell them correct path if they made a mistake */ if (uml_driver->privileged) { if (STRNEQ(conn->uri->path, "/system") && @@ -1211,13 +1218,6 @@ static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, } }
- /* URI was good, but driver isn't active */ - if (uml_driver == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("uml state driver is not active")); - return VIR_DRV_OPEN_ERROR; - } - if (virConnectOpenEnsureACL(conn) < 0) return VIR_DRV_OPEN_ERROR;
participants (3)
-
Erik Skultety -
John Ferlan -
Julio Faracco