Comment 2 for bug 432210

Revision history for this message
Padraig O'Sullivan (posulliv) wrote :

Hi!

So I had a quick look at this with a debugger at the lines of code that look to be the issue. Basically, the segfault occurs on this call:

drizzle_result_free(&databases);

The only way this method gets called is if drizzle_query_str() returns something which is not NULL. However, if we look at what the drizzle_query_str() method returns, it returns a pointer to a drizzle_result_st structure. This return value will never be NULL in this case because the actual object is passed as a parameter to the drizzle_query_str() function (the &ret parameter to this function). Thus, the if condition will always evaluate to true.

I was curious to know what the return value is after calling drizzle_query_str() and it turns out to be DRIZZLE_RETURN_COULD_NOT_CONNECT.

Now, when does that value get returned by libdrizzle? Well, it only gets returned in one place - the function drizzle_state_connect() has the following code block:

if (con->addrinfo_next == NULL)
{
  DRIZZLE_ERROR_SET(con->drizzle, "drizzle_state_connect",
                    "could not connect")
  DRIZZLE_STATE_RESET(con)
  return DRIZZLE_RETURN_COULD_NOT_CONNECT;
}

Thus, we can see that we are calling drizzle_query_str() which in turn calls drizzle_command_write() which then calls drizzle_con_connect(). This function pushes the drizzle_state_connect() function onto its state stack (which will eventually be popped off and called).

So I'm thinking a possible fix might be the following at (or around) line 2471 in client/drizzle.cc:

if (ret != DRIZZLE_RETURN_COULD_NOT_CONNECT)
{
  drizzle_result_free(&databases);
}
else
{
  return;
}

I've tried this out and it works but you might want to think a bit more on what I'm suggesting. I just had a quick look so maybe my analysis is incorrect and there is a better solution to the issue.

Another possible fix might be to change the offending block of code in client/drizzle.cc to:

/* hash all database names */
drizzle_query_str(&con, &databases, "show databases", &ret);
if (ret == DRIZZLE_RETURN_OK)
{
  if (drizzle_result_buffer(&databases) != DRIZZLE_RETURN_OK)
    put_info(drizzle_error(&drizzle),INFO_INFO,0,0);
  else
  {
    while ((database_row=drizzle_row_next(&databases)))
    {
      tmp_str= database_row[0];
      tmp_str_lower= lower_string(tmp_str);
      completion_map[tmp_str_lower]= tmp_str;
    }
  }
  drizzle_result_free(&databases);
}
else
{
  return;
}

I hope this information is useful to you.

-Padraig