-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix a incompatible pointer type warning for gcc14 #604
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same code as my second earlier test, gave same behavior as master and suppressed the warnings. I actually agree with gcc-14 treating incompatable pointers as an error by default, because my experience is code with that problem very often doesn't run right. This one with just a conflict over const seemed to work OK in master but that's the exception not the rule. Good to fix this instead of relying on compiler behavior.
Applying both this and #600 together stopped ALL the build warnings with default cflags and gcc-13 on Debian Unstable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, if that's all there is to fixing the issue 👍
Co-authored-by: Colomban Wendling <cwendling@hypra.fr>
@@ -643,7 +643,7 @@ check_mime_type(const gchar* uri,GError** error) | |||
mimeFromFile = ev_file_get_mime_type (uri, TRUE, &err); | |||
if (mimeFromFile) | |||
{ | |||
for (i = 0; i < g_strv_length (mimetypes); i++) { | |||
for (i = 0; i < g_strv_length ((gchar**) mimetypes); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you choose this solution, let's apply it here as well
for (i = 0; i < g_strv_length ((gchar**) mimetypes); i++) { | |
for (i = 0; mimetypes[i]; i++) { |
wtf, |
@raveit65 I didn't add it to the second occurrence because I didn't think it was a necessary change, but merely a possible improvement; and you can only have one suggestion per individual hunk, so I couldn't do both at once. It's no biggie, but maybe we want to harmonize this in a follow-up :) |
I pushed an update to master :-) |
@raveit65 I see :) Though, I'd rather use the same technique in both instance, rather than 2 different ones |
It was a kind of bad website automatism. After i added the first suggestion github marked/closed the conversation as resolved and i thought both suggestions were added. |
closes #603