Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix wrapped_lines variable declaration (extern)
Signed-off-by: Holger Levsen <[email protected]>
- Loading branch information
f36ec17
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.
Hi Antoni and Holger,
due to this change, GCC 10 complains about multiple storage classes in declaration of of wrapped_lines. The declaration in the header file cannot be both, static, and extern.
What was the original motivation for this change?
From a quick browse of the code, I don't see any need to maintain a shared global wrapped_lines buffer, because users of T4K_LineWrapList() provide their own output buffer.
On the other hand, it is undesirable to declare a static buffer in the common header file because every inclusion of the header file will instantiate a copy of the static buffer, even if it is not used.
I would suggest to completely remove the declaration from the common header file and let users of T4K_LineWrapList() define their own buffer variables. I'm not sure about other users of t4kcommon, but for tuxmath this would only affect campaign.c and credits.c.
I'd be happy to provide patches!
Cheers, Matthias
f36ec17
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.
f36ec17
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.
Great! What is the recommened workflow?
I wanted to create a pull request and therefore created a branch in my local clone of the repository but it seems I'm not allowed to push it:
remote: Permission to tux4kids/t4kcommon.git denied to mphilipp
f36ec17
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.
f36ec17
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.
Yes. There is no button to create a branch in the branches drop-down list.
f36ec17
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.
f36ec17
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 filed #10 which corrects the definition of this global variable, among other build fixes.