#47998 Code clean up
Closed: wontfix None Opened 6 years ago by nhosoi.

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]:

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...

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...

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...

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...

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

4 years ago

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.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

a year ago

Login to comment on this ticket.

Metadata