[OpenAFS-devel] Re: [patch] Refactoring the Solaris libafs code base

[Tom Keiser]

On 12/27/06, Sean O'Malley <omalleys@msu.edu> wrote:
>
> > > 2) Clean up src/afs/SOLARIS code.
> > >
> > I -just- submitted a patch to clean up missing string.h headers.
> > basically it either added a string.h header file or changed
>
> (but i don't think it touches much of the src/afs/SOLARIS directory..)
>
> If I need to ifdef these so they are more platform specific, I will do it.
> They are global right now and I don't want to break anything. That patch
> cuts the compiler warnings from ~4500 down to ~3750 on Solaris or at least
> it did with the recompile. I didn't test these on another platform. I just
> assume and possibly incorrectly, it might fix compiler warnings on other
> platforms also. Since I was going to ifdef the fcntl.h headers the
> same way, it is helpful to know whether I need to make these more platform
> specific.
>
> > #include <string.h> to:
> >
> > #ifdef HAVE_STRING_H
> > #include <string.h>
> > #else
> > #ifdef HAVE_STRINGS_H
> > #include <strings.h>
> > #endif
> > #endif
> >

Ok, so this is an improvement.  However, I'll point out that it's
technically incorrect for kernelspace (defined(KERNEL) &&
!defined(UKERNEL)).  For instance, the solaris kernelspace code should
be getting these prototypes out of <sys/sunddi.h>.  Your patch
attached to ticket 50400 will, for example, cause the xdr files to use
libc prototypes when we will be linking against kernel interfaces.
While this is unlikely to ever cause problems, it's still not correct.

Additionally, I'd make the point that from a software engineering
perspective, we shouldn't be sprinkling identical preprocessor logic
across hundreds of files -- It makes change management very difficult.

However, I wouldn't spend much time worrying about all of this.  Let's
just say that certain patches are scheduled to be committed to the
trunk "soon", and said patches will make it advantageous to rewrite
your patch.

-Tom