- Fixes to make ixml compiling without warnings with VS C++ (tested on VS 8)
- Removed upnpglobal dependency (proper IXML_INLINE definition needed)
- changed BOOL to IXML_BOOL to avoid conflicts
I see you did a lot of work, but we have to change a few things before all your patches can go in.
First, your patch descriptions are too short. You say "fixed warning on vc++", but you do not say which warning you are fixing. Also, you do a lot more than that. You changed BOOL for IXML_BOOL, creating a new typedef in the process. You should explain this in the commit message. And all other issues too.
Second, you should copy your commit message explaining the patch to the proper place in the Changelog file. Try to follow the writing standard used there, do not use more than 80 columns and respect the spacing. Notice that tabs are used and are equivalent to 8 spaces (linux kernel coding style). You should not leave white spaces in the end of the lines, look:
$ git am ../00*
Applying: fixed warning on vc++
/home/mr/programs/pupnp/maint/sf.git/.git/rebase-apply/patch:97: trailing whitespace.
warning: 1 line adds whitespace errors.
Applying: fixed UpnpFinish (debugout after pthread finish) fixed warnings (changed some types to size_t) fixed typos
Applying: fixed warnings (changed some types to size_t) added simple strndup implementation for windows
Applying: fixed mixed usage of SOCKET and int fixed warnings
In the ideal world, after I give the command above, everything compiles ok and I should not need to touch your patches, so it is essential that you also address the Changelog on them.
In your patch, you define EXPORT_SPEC in ixml.h. Your definition is too simplistic, this will probably break on other operating systems or even on static library compilation. This is the reason why a dependency on UpnpGlobal.h has been added. Proper solutions are either this or repeating the already defined sequence in UpnpGlobal.h inside ixml.h. I have chosen to leave the include dependency so that code is not replicated because it is unlikely that libixml will be used without libupnp. There are thousands of other XML implementations that are much better than this one, this is only included here because of embedded systems.
Another point: it would be much better to remove BOOL completely instead of creating another typedef. If you want to change BOOL all over to int, I would be more than happy to commit. Typedefs are evil, they obfuscate code instead of making it clear.
Please, try to address those issues and resubmit your patch.
I don't know what is your experience or taste with git, but if you are confortable with rebases, amending and stuff, we can work in github, the project is mirrored there too. Otherwise we can stay here in source forge exchanging git patches in the tracker, that is ok too.
Thank you for your work! I hope you keep on with us.
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).
Log in to post a comment.
Sign up for the SourceForge newsletter:
You seem to have CSS turned off.
Please don't fill out this field.