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
dev-games/t4k-common: Fix building with gcc 14 #35241
Conversation
Pull Request assignmentSubmitter: @listout dev-games/t4k-common: @gentoo/games Linked bugsBugs linked: 923789 In order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
if(xmlStrcasecmp(node->name, "menu") == 0) { | ||
i = 0; | ||
- for(child = node->children; child; child = child->next) { | ||
+ for(child = (struct _xmlAttr *)node->children; child; child = child->next) { |
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.
I'm not sure this is right. Please remember that casting is the last resort because it silences the conversion warning/error. A cast will always "work" at compile-time even if it's wrong.
It seems wrong to have to cast a _xmlNode
to a _xmlAttr
. node->children
seems like it should definitely be a node, and if we look at https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlNode, the attributes may be contained in ->properties
? But then child
is supposedly an xmlAttr
to begin with. It might need to be split into another variable to avoid treating it as two types. Go check wehere child
is first declared.
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.
From a glance, I think you may want to just use a new variable name like xmlNode childNode; childNode; childNode = childNode->next
, then fix the contents of the for
loop.
Pull request CI reportReport generated at: 2024-02-09 06:53 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Closes: https://bugs.gentoo.org/923789 Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
f823d91
to
b2a228a
Compare
@thesamesam do you think it a bit more proper now? Thanks for review BTW |
Pull request CI reportReport generated at: 2024-02-14 15:44 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
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.
Wonderful, thank you!
Please keep this lesson in mind for future, as it's applicable in other cases too.
Thanks for the good work!
@listout I noticed the upstream PR you commented on still has the old patch in it. Please update that. |
@thesamesam Done, thanks! |
Closes: https://bugs.gentoo.org/923789