Hal Patch for __u8 types

JP Rosevear jpr at novell.com
Fri Jul 15 06:36:21 PDT 2005


On Fri, 2005-07-15 at 09:33 +0100, Alvaro Lopez Ortega wrote:
> JP Rosevear wrote:
> 
>  >>+/* Make Linux types available all along the project */
>  >>+#ifdef HAVE_SYS_ASM_H
>  >>+# include <asm/types.h>
>  >>+#endif
>  >>+_______EOF
>  >>+
>  >
>  > I don't think you want to do this.  config.h can also be replaced with
>  > -D passes to the compiler, which is why people do the #ifdef
>  > HAVE_CONFIG_H thing afaik.  I think you just want to conditionally
>  > include asm/types.h in the source files themselves.
> 
>    This a compromise patch.  I mean, the perfect approach is to use
>    standard C types instead of private Linux types.  Kay complained
>    about that change because it might make harder to maintain the code,
>    so we have been looking for the best approach to make it compile
>    everywhere keeping the private Linux types.

I don't understand why you'd use standard C types, this is very OS
dependent code isn't it?

>    As far as the last patch works for everybody (it does, isn't it?)
>    I'm happy with it.  Anyway, we could also check for asm/types.h in
>    the configure script and include a #ifdef HAVE_ASM_TYPES_H in each
>    source file.
> 
>    JP, do you have some approach in mind?

I mentioned it in the last mail - break up the drive_id code into OS
dependent pieces if os dependent code is unavoidable. I'm not a hal
developer though, so this is really not up to me.

>  > This patch also doesn't solve the linux/hdreg.h problem.
>  >
>  > Perhaps the real problem is that the drive_id code isn't machine
>  > agnostic and the alternative of breaking up host specific code like in
>  > agents/hald is also not done.
> 
>    Yeah, there are more problems to compile it on OpenSolaris.  We
>    spoke about how to port those chunks of code a few days ago.  I hope
>    the patches will be available soon.

So what is the point of your alternate patch?  The patch I originally
sent changes two lines and makes everything ok on linux again.  Your
patch changes more than that, has potentially problematic config.h usage
and doesn't fully solve the solaris problem anyhow.  So what is wrong
with using my patch for now and solving the problem completely in a few
days?

-JP
-- 
JP Rosevear <jpr at novell.com>
Novell, Inc.

_______________________________________________
hal mailing list
hal at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal



More information about the Hal mailing list