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

Improve C99 compliance for wider compiler support #380

Closed
wants to merge 0 commits into from

Conversation

hashpling
Copy link
Contributor

This range of commits improves C99 compliance and as a result allows tig to be compiled with the native compilers on Solaris (SunStudio cc) and AIX (xlc).

@jonas
Copy link
Owner

jonas commented Feb 23, 2015

Wow, thanks. Nice patch set. The travis build shows two warnings which are trivial to fix. Unless you want to update the pull request, I can do these two touch ups as well as adding an entry to the NEWS file before merging.

src/prompt.c:860:21: warning: suggest braces around initialization of subobject
      [-Wmissing-braces]
                struct key key = {0};
                                  ^
                                  {}

src/main.c:96:26: warning: suggest braces around initialization of subobject
      [-Wmissing-braces]
        struct commit commit = {0};
                                ^
                                {}

@hashpling
Copy link
Contributor Author

I guess it's looking for struct commit commit = {{0}}; or even {""};. To me, plain {0} is idiomatic C and "double bracing" looks a bit ugly. Note that this warning should go away in a newer version of gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119 .

@hashpling
Copy link
Contributor Author

So I tested with {{0}} in both places and both native compilers are happy so it's up to you. If a warning-free travis is important to you then I don't object to these touch ups even though I think it's a bogus warning in this case.

@jonas
Copy link
Owner

jonas commented Feb 23, 2015

What is important is that make all-debug which adds -Wall to CFLAGS works for me locally and it does not. So I will add the missing braces.

@jonas
Copy link
Owner

jonas commented Feb 23, 2015

Thanks merged.

It's probably time to embrace more C99 constructs in the code base, especially for struct initialization. This is a good start!

@hashpling
Copy link
Contributor Author

Thanks! I'll test with all-debug, at least on Linux, for any further patches which I come up with.

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.

None yet

2 participants