[RFC,02/15] qobject: allow NULL for qstring_get_str()

Message ID 1505375436-28439-3-git-send-email-peterx@redhat.com
State New
Headers show
Series
  • QMP: out-of-band (OOB) execution support
Related show

Commit Message

Peter Xu Sept. 14, 2017, 7:50 a.m.
Then I can get NULL rather than crash when calling things like:

  qstring_get_str(qobject_to_qstring(object));

when key does not exist.

CC: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 qobject/qstring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Blake Sept. 19, 2017, 8:48 p.m. | #1
On 09/14/2017 02:50 AM, Peter Xu wrote:
> Then I can get NULL rather than crash when calling things like:
> 
>   qstring_get_str(qobject_to_qstring(object));
> 
> when key does not exist.

Right now, qdict_get_str() is documented as:

 * This function assumes that 'key' exists and it stores a
 * QString object.

Your code changes that, by making it now return NULL instead of crashing
on what used to be usage in violation of the contract; compared to
qdict_get_try_str() which gracefully returns NULL, but which could use
your new semantics for doing so in fewer lines of code.

I'm not necessarily opposed to the change, but I worry that it has
subtle ramifications that we haven't thought about, as well as
consistency with the rest of the QObject APIs.  It may be better to just
introduce qstring_get_try_str(), which gracefully handles NULL input,
and leave the existing function alone; and if you do introduce a new
helper, it may be worth converting existing clients (perhaps with help
from Coccinelle) to take advantage of the helper.

> +++ b/qobject/qstring.c
> @@ -125,7 +125,7 @@ QString *qobject_to_qstring(const QObject *obj)
>   */
>  const char *qstring_get_str(const QString *qstring)
>  {
> -    return qstring->string;
> +    return qstring ? qstring->string : NULL;
>  }
>  
>  /**
>

Patch

diff --git a/qobject/qstring.c b/qobject/qstring.c
index 5da7b5f..c499fec 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -125,7 +125,7 @@  QString *qobject_to_qstring(const QObject *obj)
  */
 const char *qstring_get_str(const QString *qstring)
 {
-    return qstring->string;
+    return qstring ? qstring->string : NULL;
 }
 
 /**