On 12/27/06, Sean O'Malley <[EMAIL PROTECTED]> 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
_______________________________________________
OpenAFS-devel mailing list
[email protected]
https://lists.openafs.org/mailman/listinfo/openafs-devel

Reply via email to