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

Use integer port argument with mysql_real_connect #3325

Merged
merged 2 commits into from Jan 5, 2024

Conversation

fweimer-rh
Copy link
Contributor

The port argument is not a string, but a plain integer. In the past, compilers ignore such type errors and produced a warning only, but this is changing, so this change also addresses a potential build failure.

Note that I can't really test this, but at least the compiler error is gone. Found as part of:

The port argument is not a string, but a plain integer.  In the past,
compilers ignore such type errors and produced a warning only, but
this is changing, so this change also addresses a potential build
failure.
@github-actions github-actions bot added C Related code is in C database Related to database management module labels Jan 3, 2024
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

This is consistent with mysql_real_connect and db_get_login2 which creates the string.

Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

Thanks @fweimer-rh !
mysql_real_connect() does indeed use unsigned int for the port (in comparison to PQsetdbLogin() which uses char *).

@wenzeslaus
Copy link
Member

Anyone has a trick for quickly fixing clang-format issues without pulling the code locally? Unlike with Black, I'm not good at guessing what the right formatting is from the message.

db/drivers/mysql/db.c Outdated Show resolved Hide resolved
@echoix
Copy link
Member

echoix commented Jan 4, 2024

Its ugly. I wasn't able to create a good suggestion.

@neteler
Copy link
Member

neteler commented Jan 4, 2024

(For the record for those not familiar with gh: it is fairly straighforward)

gh pr checkout 3325
utils/grass_clang_format.sh db/drivers/mysql/db.c
git diff
git commit -m"utils/grass_clang_format.sh db/drivers/mysql/db.c" db/drivers/mysql/db.c
git push 

Done in fc9803f

@nilason
Copy link
Contributor

nilason commented Jan 4, 2024

Anyone has a trick for quickly fixing clang-format issues without pulling the code locally? Unlike with Black, I'm not good at guessing what the right formatting is from the message.

#3255 ;-)

@nilason
Copy link
Contributor

nilason commented Jan 4, 2024

I believe we have no CI test with MySQL driver, I’ll look into this. I suppose this should have been caught.

@wenzeslaus wenzeslaus merged commit 3a9db16 into OSGeo:main Jan 5, 2024
20 checks passed
@wenzeslaus
Copy link
Member

[The formatting is] ugly. I wasn't able to create a good suggestion.

I agree. We didn't necessarily choose the format. We used whatever was already there and what clang-format supported at the time hoping to get back to creating a nicer style with a more powerful version of clang-format.

@nilason
Copy link
Contributor

nilason commented Jan 5, 2024

[The formatting is] ugly. I wasn't able to create a good suggestion.

I agree. We didn't necessarily choose the format. We used whatever was already there and what clang-format supported at the time hoping to get back to creating a nicer style with a more powerful version of clang-format.

Attractive or not, the point of using formatters like ClangFormat and Black is to avoid non-productive discussions on formatting with potentially no end ...... :)

@wenzeslaus
Copy link
Member

...the point of using formatters like ClangFormat and Black is to avoid non-productive discussions on formatting with potentially no end ...... :)

Again, I deny ever taking a part in such discussion.

HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
The port argument is not a string, but a plain integer.  In the past, compilers ignore such type errors and produced a warning only, but this is changing, so this change also addresses a potential build failure.

Unlike  PQsetdbLogin() which uses char *, mysql_real_connect() uses unsigned int for the port.

Found as part of Porting To Modern C in the Fedora Project.
@neteler neteler added this to the 8.4.0 milestone Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C database Related to database management module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants