[Bug 27167] Gabble MailNotification.RequestInboxURL returns empty string

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=27167

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
  Status Whiteboard|                            |review-, minor changes
         AssignedTo|telepathy-bugs at lists.freede |nicolas.dufresne at collabora.
                   |sktop.org                   |co.uk
          QAContact|                            |telepathy-bugs at lists.freede
                   |                            |sktop.org

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-04-12 10:03:15 PDT ---
> +  GList *pending_requests;

This needs a comment about what its contents are (/* list of
DBusGMethodInvocation */ is sufficient), and should be called
inbox_url_requests or something (since it doesn't contain enough context to be
used for anything else).

> +  if (priv->inbox_url != NULL && priv->inbox_url[0] == 0)
> +      {
> +        error = g_error_new (TP_ERRORS, TP_ERROR_NETWORK_ERROR,
> +            "Failed to obtain base URL.");
> +      }
> +  else if (priv->inbox_url == NULL)
> +      {
> +        error = g_error_new (TP_ERRORS, TP_ERROR_NETWORK_ERROR,
> +            "Failed to obtain base URL.");
> +      }

What's the point of doing this? Just have one check, using the fact that ||
short-circuits:

  if (priv->inbox_url == NULL || priv->inbox_url[0] == '\0')

or if your intention was to have different error messages/codes in the two
cases, do that :-)

(I also prefer the zero'th char to be spelled '\0' rather than 0, as seen in my
suggested version, even though strictly speaking they're equivalent.)

> +  if (priv->inbox_url)
> +    return_from_request_inbox_url (conn);

if (priv->inbox_url != NULL)

> +          "Failed to retreive URL from server."};

retrieve

> +  /* Use empty string to differientiate pending request from server failing to

differentiate

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.