Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug when I trying install the mariadb with pip #29

Draft
wants to merge 2 commits into
base: 1.1
Choose a base branch
from

Conversation

loki0b
Copy link

@loki0b loki0b commented May 11, 2024

I was not able to install mariadb-connector with pip because it was getting a compiler error when building.
Probably because one of the flags -Werror=format-security which treats warning as error.

The output of error:

mariadb/mariadb_cursor.c: In function ‘MrdbCursor_execute_text’:
  mariadb/mariadb_cursor.c:1138:39: error: passing argument 2 of ‘PyBytes_AsStringAndSize’ from incompatible pointer type [-Wincompatible-pointer-types]
   1138 |         PyBytes_AsStringAndSize(stmt, &statement, (Py_ssize_t *)&statement_len);
        |                                       ^~~~~~~~~~
        |                                       |
        |                                       const char **
  In file included from /usr/include/python3.12/Python.h:50,
                   from ./include/mariadb_python.h:21,
                   from mariadb/mariadb_cursor.c:20:
  /usr/include/python3.12/bytesobject.h:56:12: note: expected ‘char **’ but argument is of type ‘const char **’
     56 |     char **s,           /* pointer to buffer variable */
        |     ~~~~~~~^
  error: command '/usr/bin/gcc' failed with exit code 1

Since the function PyBytes_AsStringAndSize from bytesobjects.h expectate a char** and not a const char** I remove the "pass by reference" and add a explicit conversion of type of statement to (char**).

After that I could install the mariadb package with pip using the source code that I edit.

@vkabadzhova
Copy link

Hey, there! Same issue here. Do you need to recompile the c files somehow after the change? Please, specify the exact steps you went through:) Thanks in advance.

@loki0b
Copy link
Author

loki0b commented May 21, 2024

Yes. It needs to you install the package from source.
After make this changes above just go to the package root then run pip install .

@@ -1135,7 +1135,7 @@ MrdbCursor_execute_text(MrdbCursor *self, PyObject *stmt)
statement = PyUnicode_AsUTF8AndSize(stmt, (Py_ssize_t *)&statement_len);
} else if (Py_TYPE(stmt) == &PyBytes_Type)
{
PyBytes_AsStringAndSize(stmt, &statement, (Py_ssize_t *)&statement_len);
PyBytes_AsStringAndSize(stmt, (char**)statement, (Py_ssize_t *)&statement_len);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the address (&) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello. I remove it to convert directly to char**.
I tried changing the statement type from const char* to char* and it also worked.
However, I don't know what is the best way to do it and whether the latter could cause problems.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of casting away the const incorrectly, within this function you can instead drop const from the declaration of statement:

diff --git a/mariadb/mariadb_cursor.c b/mariadb/mariadb_cursor.c
index 65fab74..191f93a 100644
--- a/mariadb/mariadb_cursor.c
+++ b/mariadb/mariadb_cursor.c
@@ -1125,7 +1125,7 @@ MrdbCursor_execute_text(MrdbCursor *self, PyObject *stmt)
 {
     int rc;
     MYSQL *db;
-    const char *statement;
+    char *statement;
     size_t statement_len= 0;
 
     MARIADB_CHECK_CONNECTION(self->connection, NULL);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey. I made these changes and updated the pr. Thanks :)

…ation

I made this change based on pr comments.
I removed the const key when declaring statement and reverted the changes I had made previously (explicitly casting the statement variable to char**)
@tkuschel
Copy link

When you remove const from statement, you will break the line with PyUnicode_AsUTF8AndSize():

statement = PyUnicode_AsUTF8AndSize(stmt, (Py_ssize_t *)&statement_len);

That funciton is defined as

const char *PyUnicode_AsUTF8AndSize(PyObject *unicode, Py_ssize_t *size)

seen in https://docs.python.org/3/c-api/unicode.html.
This returns a const char *. And this throws a warning in the upper removing of const from statement.

Also the function `mysql_send_query(db, statement, (long)statement_len) is defined as

int mysql_send_query(MYSQL * mysql,
                     const char * query,
                     unsigned long length);

So either we cast the &statement to a line like

        PyBytes_AsStringAndSize(stmt, (char **)&statement, (Py_ssize_t *)&statement_len);

or for more readability:

    } else if (Py_TYPE(stmt) == &PyBytes_Type)
    {
        char *sbuf;
        PyBytes_AsStringAndSize(stmt, &sbuf, (Py_ssize_t *)&statement_len);
        statement = sbuf;
    }

@tkuschel
Copy link

Within this function, we can also exchange the size_t with Py_ssize_t, then we have less castings.

static PyObject *
MrdbCursor_execute_text(MrdbCursor *self, PyObject *stmt)
{
    int rc;
    MYSQL *db;
    const char *statement;
    Py_ssize_t statement_len= 0;

    MARIADB_CHECK_CONNECTION(self->connection, NULL);

    if (Py_TYPE(stmt) == &PyUnicode_Type)
    {
        statement = PyUnicode_AsUTF8AndSize(stmt, &statement_len);
    } else if (Py_TYPE(stmt) == &PyBytes_Type)
    {
        PyBytes_AsStringAndSize(stmt, (char **)&statement, &statement_len);
    }
    else {
        PyErr_SetString(PyExc_TypeError, "Parameter must be a string or bytes");
        return NULL;
    }
    db= self->connection->mysql;

    MARIADB_BEGIN_ALLOW_THREADS(self->connection);
    rc= mysql_send_query(db, statement, (long)statement_len);
    MARIADB_END_ALLOW_THREADS(self->connection);

    if (rc)
    {
        mariadb_throw_exception(db, NULL, 0, NULL);
        return NULL;
    }
    Py_RETURN_NONE;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants