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

unbalanced #endif's #663

Closed
Begasus opened this issue Dec 21, 2017 · 29 comments
Closed

unbalanced #endif's #663

Begasus opened this issue Dec 21, 2017 · 29 comments

Comments

@Begasus
Copy link

Begasus commented Dec 21, 2017

When trying to build htmltidy on Haiku for gcc2 (v2.95.) I'm getting some unbalanced #endif's for the language_ header files, for gcc5* it's ok, any way to fix these?
PS, this is for latest version 5.6.0

@Begasus
Copy link
Author

Begasus commented Dec 21, 2017

@Begasus
Copy link
Author

Begasus commented Dec 21, 2017

build output: https://pastebin.com/4Zu6tU7U

@geoffmcl
Copy link
Contributor

@Begasus well reading around it seems Haiku gcc2 2.95 was released circa 2001, now 16 years old, but really that does not seem to explain the unbalanced #endif in the language_* header files, that later compilers do not find...

The earliest I have for testing is gcc-4.8 and it compiles without errors, or warnings, as do a few other later versions I have...

It is almost as if the 2.95 compiler skips the first line in each of these files...

They all start with an #ifndef ..., as the very first line... so you could try adding a newline, or comment like /* nl */, at the beginning of each... test and report...

But then I note you show another error in tidylib.c, namely bomEnc is undefined. I have found at least one old compiler, MSVC10, that likewise flags this error. But it is fixed by moving its declaration up one line, to before the doc->pConfigChangeCallback = NULL...

While we may consider the second change for bomEnc, I do not know what we can do about the first errors... it may be a case where you must apply your own local patches to use this old 2.95 compiler...

And some of the warnings are strange, like messageobj.h:66: warning: parameter has incomplete type, repeated several times, but as usual they may not actually be a problem...

HTH... and look forward to your further feedback... thanks...

@geoffmcl geoffmcl added this to the 5.7 milestone Dec 21, 2017
@Begasus
Copy link
Author

Begasus commented Dec 21, 2017

@geoffmcl thanks for the reply, will check later and report back 👍

@geoffmcl
Copy link
Contributor

@Begasus since tidy is intended as a simple pure C project, in essence it should be compilable by all versions of gcc...

And would welcome any PR to make it so...

Look forward to your further feedback... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 22, 2017

@geoffmcl for the unbalanced #endif it was ok to remove the whitespace in front of the first line, still need to investigate the other errors, thanks for the pointer!

@geoffmcl
Copy link
Contributor

@Begasus ah, ha, thanks for the further feedback...

On more carefully checking I find 6 source files have a UTF-8 BOM - language_en_gb.h, language_es.h, language_es_mx.h, language_fr.h, language_pt_br.h, language_zh_cn.h - that is they start with hex ef bb bf, and I guess that is what you are referring to as whitespace in front of the first line...

And I guess that is what gcc 2.95 trips on...

Reading around it seems adding a UTF-8 BOM is not necessary, and see some suggestions that it should not be added, as it can cause problems with older software...

These language files were added by @balthisar, and maybe he could comment on its possible removal... I think it is being added by a generation tool he uses... thanks...

I will try to get around to removing it just to check my windows compilers will have not problems with utf-8 without it...

Look forward to further feedback on the other errors... thanks...

geoffmcl added a commit that referenced this issue Dec 23, 2017
@geoffmcl
Copy link
Contributor

@Begasus, @balthisar, just a quick follow-up...

I made the small change for bomEnc, and removed the BOM from those 6 files, and everything built fine using MSVC10 and MSVC14, in both 32-bits and 64-bits...

Have pushed this to the issue-663 branch for testing in all environments... Note this issue-663 is based on next, version 5.7.0, but at this time this is the same as release 5.6.0 aside from this version number...

Look forward to further feedback... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 23, 2017

@geoffmcl thanks!, will check with the latest commit :)

@geoffmcl
Copy link
Contributor

Have now had a chance to build issue-663 branch in -

  1. Ubuntu 14.04 64-bit, gcc 4.8.4, no errors or warnings - clean build
  2. Raspbian 8, 32-bit, gcc 4.9.2, no errors or warnings - clean build
  3. Ubuntu 16.04, 32-bit, gcc 5.4.0, no errors or warnings - clean build

Still to light up my OLD Windows XP system, and try with MSVC8 2005 - will get to that - but expect no errors, but a language*.h warning about the #pragma execution_character_set("utf-8"), which it seems was not added until a later MSVC version...

All this seems to suggest the UTF-8 BOM can be safely removed...

@Begasus hope you have good luck with gcc 2.95, or any other compilers you have... please advise when you have time...

@Begasus
Copy link
Author

Begasus commented Dec 23, 2017

@geoffmcl this issue can be closed as far for Haiku, no unbalanced #endif's now 👍
And works ok also for gcc5* here ..
Only error I run into now with gcc 2.95 is

/sources/tidy-html5-7af9e1843b0ddcb96d235ee9a2f73eb6630f7227/src/tidylib.c:1138: parse error before `struct'

/sources/tidy-html5-7af9e1843b0ddcb96d235ee9a2f73eb6630f7227/src/tidylib.c:1142: `sbuf' undeclared (first use in this function)

But maybe I could add a different issue for that one?

@Begasus
Copy link
Author

Begasus commented Dec 23, 2017

hmm .. maybe wait a bit to close this one ... haven't had any chance to fix the parse error ...

@Begasus
Copy link
Author

Begasus commented Dec 23, 2017

Not sure if it's related ...

/sources/tidy-html5-7af9e1843b0ddcb96d235ee9a2f73eb6630f7227/src/language.c:44: invalid initializer

@geoffmcl
Copy link
Contributor

@Begasus re tidylib.c:1138: parse error before 'struct'...

Well, it seems the same issue, namely compile with OLD gcc 2.95... so think we can stay here...

That usually indicates <sys/types.h> and <sys/stat.h> have not been included... but they should be included through the chain #include "tidy-int.h" -> #include "tidy.h" -> #include "tidyplatform.h... you need to check that they are indeed included... but they should be...

Alternatively, struct stat is defined somewhere else in Haiku... but it is weird in that you advise gcc5* works...

AHHH, HAAA!, but you had already given a BIG clue when you had a problem with bomEnc... seems gcc 2.95 strictly enforces the K&R rule that in any given context all variables must be declared before any code...

The following code breaks that rule...

    fin = fopen( filnam, "rb" );    /* this is CODE */

#if PRESERVE_FILE_TIMES
    struct stat sbuf = {0};         /* new VARIABLES can **NOT** be declared here */
    /* get last modified time */
    TidyClearMemory( &doc->filetimes, sizeof(doc->filetimes) );
    ...

You can probably overcome it by openning a new context, like -

    fin = fopen( filnam, "rb" );

#if PRESERVE_FILE_TIMES
    {   /* open new context for K&R */
        struct stat sbuf = {0};
        /* get last modified time */
        TidyClearMemory( &doc->filetimes, sizeof(doc->filetimes) );
        ...
        ...
    }   /* close new context */
#endif

Or move the struct stat sbuf; declaration to the top of the function... leaving only sbuf = {0}; here...

Most, if not all!, later versions of gcc allow this K&R rule to be broken all the time... much to my chagrin, since this K&R rule is also enforced by MSVC10, and earlier...

There may be other places where this happens in unix only code, since todate I have found no way to instruct later gcc versions to flag this error...

They have all been fixed in windows only, or shared code, since my MSVC10 will bark if this is done wrong order - that is NOT all variable before code, in any given context...

Let us know if this K&R change works for you... thanks...

Still to look at src/language.c:44: invalid initializer... but that will probably be tomorrow now...

@geoffmcl
Copy link
Contributor

@Begasus re: src/language.c:44: invalid initializer

Took a quick look... but does not seem bad initialization code for a complex structure, and assume you see no problems with gcc5* builds...

One immediate question - is this only line 44, or is it flagging all of them, including the last NULL entry, line 54?

You will need to mess with it a little, and try to find what gcc 2.95 likes! There must be a way to get this structure statically setup... but can't think of anything at the moment...

Anyway, off for dinner... be back soonest...

@Begasus
Copy link
Author

Begasus commented Dec 23, 2017

Putting the {} in worked it seems for the parse error, still stuck with the language.c error ...
https://pastebin.com/7JeU6SmH

@Begasus
Copy link
Author

Begasus commented Dec 24, 2017

@geoffmcl

That usually indicates <sys/types.h> and <sys/stat.h> have not been included... but they should be included through the chain #include "tidy-int.h" -> #include "tidy.h" -> #include "tidyplatform.h... you need to check that they are indeed included... but they should be...

There is no: tidy.h nor tidyplatform.h in the source?

Strike that, found it, but didn't change anything by adding the headers there ...

@Begasus
Copy link
Author

Begasus commented Dec 24, 2017

Working with these changes ...
tidy_git-5.7.0.patchset.txt

@geoffmcl
Copy link
Contributor

@Begasus thank you for pushing forward on this... it seems we are adding a new port, namely Haiku, which I must admit, I had never heard of before this post... using their gcc 2.95, 32-bit... this is a good thing...

Have looked at your patchset.txt and feel that all in here can be added to my issue-663 branch, which will eventually be merged to next, and maybe backported to the 5.6.0 release... namely -

  1. tidylib.c - add a context around the #if PRESERVE_FILE_TIMES
  2. tidyplatform.h - these changes all look good, to show it as an Haiku build... #if defined(__HAIKU__)
  3. language.c - you mean the compiler has no problem if we add a [256] to the structure?

While I am not particularly in love with the arbitrary 256 limit on languages, think I can wear this... unless someone suggests another solution, like say a supported language enum which sets a value for this...

I do understand, back 20 years ago, a compiler hating an open memory allocation like []... it was not so long ago the MSVC compilers would also bark at this...

Are you saying, adding these to the other fixes, gives you a clean build in Haiku, using gcc 2.95?

That is have we addressed all the changes required? ... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 24, 2017

@geoffmcl I think we can say all is dealt with now, issue could be closed (unless someone else steps up) 👍

For x86_gcc2 (gcc2 compiler)

~/haikuports> pkg-config tidy --cflags
-I/packages/tidy_git-5.7.0-1/.self/develop/headers
~/haikuports> pkg-config tidy --libs
-L/packages/tidy_git-5.7.0-1/.self/develop/lib -ltidy
~/haikuports> tidy -v
HTML Tidy for Haiku version 5.7.0

@Begasus
Copy link
Author

Begasus commented Dec 24, 2017

For x86 (gcc5) (also working for x86_64, 64bit)

~/haikuports> pkg-config-x86 tidy --cflags
-I/packages/tidy_git_x86-5.7.0-1/.self/develop/headers/x86
~/haikuports> pkg-config-x86 tidy --libs
-L/packages/tidy_git_x86-5.7.0-1/.self/develop/lib/x86 -ltidy
~/haikuports> tidy -v
HTML Tidy for Haiku version 5.7.0

@Begasus
Copy link
Author

Begasus commented Dec 24, 2017

@geoffmcl Thanks for all the help (and they guys @haiku for their help) to get this fixed! :)
Happy Hollidays!

@geoffmcl
Copy link
Contributor

@Begasus thank you for the report...

Have pushed all the changes to the issue-663 branch...

But had to do it manually since could not get your patch to work with my windows port of patch, but have had this problem before, so would appreciate you pulling that branch again, and test compile again to make sure I did not make some silly mistake... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 25, 2017

@geoffmcl Thanks for adding this! Builds without patching now! 👍

https://pastebin.com/hUVwPms6

@Begasus
Copy link
Author

Begasus commented Dec 25, 2017

From one of the developers on Haiku's side: haikuports/haikuports#1932 (comment)
Maybe you can look at it and make sure the patch doesn't break anything for tidy ...

@geoffmcl
Copy link
Contributor

@Begasus thank you for testing the issue-663 branch...

As @fbrosson noted, have reduced the 256 to 8 in the above push... and adjusted the comment accordingly... absolutely no need to over-allocate the static memory for this structure...

But your results still shows many warnings -

  1. .../src/messageobj.h:66: warning: parameter has incomplete type - many of these
  2. .../src/config.c:1427: warning: 'ParseName' defined but not used - is marked FUNC_UNUSED, so ok...
  3. .../src/tidylib.c:969: warning: integer overflow in expression - a few of these
  4. .../console/tidy.c:859: warning: field width is not type int (arg 3)

I think the multiple 1. warning are due to a dereferenced pointer being placed on the stack. It allows the structure members to be addressed using the dot, a.b notation, rather than passing a pointer which must be referenced as a->b...

Maybe older compilers have no way to do this, since this is a relatively new C++ like feature that has crept into some C... and maybe those tidy API functions would be broken in the Haiku port. Do not know...

But they are all relatively new API functions, dealing only with heavy message filtering, and are probably not so important in that they would be quite unlikely to be used in any older application linking with libtidy... They are not used by the console app, tidy, so should cause no trouble for it...

If I am right, the only solution would be to re-write those functions using a pointer, and the normal a->b notation... or cut those API's from the Haiku port... but maybe not required...

The number 2. is indeed an unused function, and is marked with a FUNC_UNUSED macro, which is defined as an attribute in __GNUC__, of #define FUNC_UNUSED __attribute__((unused)), which probably also would not be available in gcc2 2.95... But it can be ignored without consequence... just some unused dead code, which someday should in fact be removed.

As to 3., can not see why return ( impl->errout ? 0 : -ENOMEM ); on functions that returns an int is a problem... does it not like -ENOMEM... or what? Maybe could be fixed with an return (int)(...) cast... Anyway seems it can be ignored...

And 4. is the line printf( "%*.*s\n\n", width, width, ul);, which just prints the underline in the --help output. Does 2.95 not understand the format %*.*s? Or what? You should do $ tidy --help and see what you get? The ul should be a pointer to the string static const char ul[] = "====etc";, and certainly not an int! But if the --help output is ok, then it can be ignored, else may need some fix for #ifdef __HAIKU__ or something...

And another warning noted is that from cmake -

-- *** NOTE: xsltproc NOT FOUND! Can NOT generate man page.
-- *** You need to install xsltproc in your system.

This app is used to generate the tidy.1 man page, which should be part of the distribution package, if Haiku supports $ man tidy...

Other than that, all looks good... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 25, 2017

xsltproc wasn't included in the recipe yet, it is now and man page is installed, thanks! If there are no problems on your side this one can be closed, thanks for taking the time to investigate into this, really appreciated! 👍

@geoffmcl
Copy link
Contributor

@Begasus just glad I could help with a new Haiku port of tidy... will create a PR for the branch, and in due course it will be merged to next...

And will give thought to back porting it, together with some others, into 5.6, potentially creating a 5.6.1 release...

Still curious how the --help looks... but no problem...

As suggested, will close this... thanks...

@Begasus
Copy link
Author

Begasus commented Dec 26, 2017

Per request ;)

https://pastebin.com/zLf5yx3E

geoffmcl added a commit that referenced this issue Jan 1, 2018
Issue 663 - fixes for Haiku port - closes #663
geoffmcl added a commit that referenced this issue Jan 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants