Remove obsolete #ifdef.
E.g., "#ifdef _WIN32" or XP_WIN32. File list is attached.
Files that contain "_WIN32". win32.txt
There are also references to AIX and IRIX thatshould probably be rmeoved. There is a dllmain file for every plugin, I think these can be removed as well. There are other "debug" files that were windows specific that can probably be removed too.
Putting "phase 1" patch out for review (remove windows #ifdefs). Then I will work on the AIX/IRIX #ifdefs, and the windows NT file removal.
1) Replying to [comment:5 mreynolds]:
There is a dllmain file for every plugin, I think these can be removed as well.
I agree! They are not compiled at all... $ egrep dllmain Makefile.am $
When I reviewed, I ignored them. ;)
2) We also don't need .def files like this, do we? ldap/servers/slapd/libslapd.def
3) I noticed we are removing "#ifdef XP_UNIX" and make the code always compiled. Are we getting rid of the macro from config.h.in and configure.ac, too? $ egrep XP_UNIX config.h.in configure.ac config.h.in:#undef XP_UNIX configure.ac: AC_DEFINE([XP_UNIX], [1], [UNIX]) ....
These files have the ifdef XP_UNIX... $ find . -name "*.[ch]" | xargs egrep -l XP_UNIX ./ldap/servers/slapd/tools/rsearch/searchthread.c ./ldap/servers/slapd/tools/rsearch/addthread.c ./ldap/servers/slapd/tools/rsearch/main.c ./ldap/servers/slapd/tools/rsearch/rsearch.c ./include/netsite.h ./include/libadmin/libadmin.h ./include/base/fsmutex.h ./include/base/systems.h ./include/public/nsapi.h ./lib/libadmin/error.c ./lib/libadmin/util.c ./lib/ldaputil/init.c
Replying to [comment:6 nhosoi]:
3) I noticed we are removing "#ifdef XP_UNIX" and make the code always compiled. Are we getting rid of the macro from config.h.in and configure.ac, too? $ egrep XP_UNIX config.h.in configure.ac
Nice catch! I left most of the XP_UNIX intact, but I must have removed some by accident. I'll get this cleared up and resend the patch. Then in separate patches I'll address the other issues (dllmain files, etc)
Replying to [comment:7 mreynolds]:
Replying to [comment:6 nhosoi]: 3) I noticed we are removing "#ifdef XP_UNIX" and make the code always compiled. Are we getting rid of the macro from config.h.in and configure.ac, too? $ egrep XP_UNIX config.h.in configure.ac Nice catch! I left most of the XP_UNIX intact, but I must have removed some by accident. I'll get this cleared up and resend the patch. Then in separate patches I'll address the other issues (dllmain files, etc)
Sorry I misread your comment, I'll remove the macro completely...
I don't think we want to remove all #ifdef XP_UNIX.
Replying to [comment:9 rmeggins]:
Yeah, after looking into this a bit more I agree. Originally I thought it was just another way to distinguish UNIX from Windows, but I don't think that's the case. Still investigating this "XP_UNIX" macro...
Replying to [comment:10 mreynolds]:
Replying to [comment:9 rmeggins]: I don't think we want to remove all #ifdef XP_UNIX. Yeah, after looking into this a bit more I agree. Originally I thought it was just another way to distinguish UNIX from Windows, but I don't think that's the case. Still investigating this "XP_UNIX" macro...
In some cases it means "as opposed to linux", not just "as opposed to windows".
I see. Thanks, Rich and Mark.
How about my question on "connection_reset (connection.c)"? Once it's confirmed, I will ack this ticket.
Thanks!
Replying to [comment:11 rmeggins]:
Replying to [comment:10 mreynolds]: Replying to [comment:9 rmeggins]: I don't think we want to remove all #ifdef XP_UNIX. Yeah, after looking into this a bit more I agree. Originally I thought it was just another way to distinguish UNIX from Windows, but I don't think that's the case. Still investigating this "XP_UNIX" macro... In some cases it means "as opposed to linux", not just "as opposed to windows".
Actually, we set this for LINUX (as well as Solaris, and HPUX) -> see configure.ac. So it appears it only means NON-Windows.
I see no reason to keep XP_UNIX after further investigation, new patch attached.
Need to revise patch for change in connection.c...
revision #2 - remove XP_UNIX and revise connection.c 0001-Ticket-47998-cleanup-WINDOWS-ifdef-s.patch
New patch attached.
Thank you, Mark! (I wonder how many lines you could get rid of...? ;)
e6a65c8..003651c master -> master commit 003651c Author: Mark Reynolds mreynolds@redhat.com Date: Mon Jun 15 12:03:25 2015 -0400
Moving on to removing windows specific source and def files, etc...
Remove dllmain and definition files 0001-Ticket-47998-remove-windows-files.patch
Thanks Noriko!
538ff19..3c0d3c5 master -> master commit 3c0d3c5 Author: Mark Reynolds mreynolds@redhat.com Date: Mon Jun 15 13:20:50 2015 -0400
Working AIX and IRIX code removal next...
remove the remining obsolete OS code/files 0001-Ticket-47998-remove-remaining-obsolete-OS-code-files.patch
065b511..d42a531 master -> master commit d42a531 Author: Mark Reynolds mreynolds@redhat.com Date: Tue Jun 16 09:34:25 2015 -0400
Metadata Update from @mreynolds: - Issue assigned to mreynolds - Issue set to the milestone: 1.3.4.0
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/1329
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.