qapi: Clean up fragile use of error_is_set()
Using error_is_set(ERRP) to find out whether a function failed is either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP may be null, because errors go undetected when it is. It's fragile when proving ERRP non-null involves a non-local argument. Else, it's unnecessarily opaque (see commit 84d18f0). The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because the caller never passes a null errp argument. Make the code more robust and more obviously correct: receive the error in a local variable, then propagate it through the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This commit is contained in:
		
							parent
							
								
									196857f8bf
								
							
						
					
					
						commit
						ee16ce9337
					
				@ -62,6 +62,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
static QObject *do_qmp_dispatch(QObject *request, Error **errp)
 | 
					static QObject *do_qmp_dispatch(QObject *request, Error **errp)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
 | 
					    Error *local_err = NULL;
 | 
				
			||||||
    const char *command;
 | 
					    const char *command;
 | 
				
			||||||
    QDict *args, *dict;
 | 
					    QDict *args, *dict;
 | 
				
			||||||
    QmpCommand *cmd;
 | 
					    QmpCommand *cmd;
 | 
				
			||||||
@ -93,14 +94,14 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    switch (cmd->type) {
 | 
					    switch (cmd->type) {
 | 
				
			||||||
    case QCT_NORMAL:
 | 
					    case QCT_NORMAL:
 | 
				
			||||||
        cmd->fn(args, &ret, errp);
 | 
					        cmd->fn(args, &ret, &local_err);
 | 
				
			||||||
        if (!error_is_set(errp)) {
 | 
					        if (local_err) {
 | 
				
			||||||
            if (cmd->options & QCO_NO_SUCCESS_RESP) {
 | 
					            error_propagate(errp, local_err);
 | 
				
			||||||
 | 
					        } else if (cmd->options & QCO_NO_SUCCESS_RESP) {
 | 
				
			||||||
            g_assert(!ret);
 | 
					            g_assert(!ret);
 | 
				
			||||||
        } else if (!ret) {
 | 
					        } else if (!ret) {
 | 
				
			||||||
            ret = QOBJECT(qdict_new());
 | 
					            ret = QOBJECT(qdict_new());
 | 
				
			||||||
        }
 | 
					        }
 | 
				
			||||||
        }
 | 
					 | 
				
			||||||
        break;
 | 
					        break;
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user