nanogui: Thread: Use of "unsigned long" for 32-bit quantities

I've noticed in some parts of microwindows/nano-X, "unsigned long" is used 
to mean "unsigned 32-bit value".

On my platform's compiler, "unsigned long" is > 32 bits (I think it's 64 
bits - I don't remember), so things in microwindows/nano-X 'break'.  For 
example, in "engine/font_pcf.c", in "pcf_createfont()" ,there's an "unsigned 
long *ptr" that's used as a pointer into the bitmap data.  "(xwidth + 1) / 
2" is added to it, with the assumption that sizeof(unsigned long) is 4 
bytes.  When sizeof(unsigned long) is 8, all offsets into the bitmaps are 
screwed-up.

I usually use "unsigned int" to mean 32-bit value, but I know that some 
smaller embedded compilers use 16 bit integers (so, I assume that's the 
reason "unsigned long" is used).

I suggest a global header file with 'generic' typedefs (like "uint16", 
"uint32", etc), which can be customized based on your compiler.  Then, 
instead of using types like "unsigned long" and "unsigned int" throughout 
the code (when the code really means: 32bits or 16 bits), use the new 
"uint32/uint16" types.

Does this seem like a good idea?  If so, how difficult would it be to get 
done?  (I know that I could do it, but others probable know the microwindows 
code a lot better than I do, and could do it more quickly).  If nobody else 
volunteers, and nobody *objects*, I'll go ahead and make the changes.

I'd appreciate feedback.

Thanks,

- Paul Bartholomew

On Thursday 30 June 2005 21:44, Paul Bartholomew wrote:
...
> I suggest a global header file with 'generic' typedefs (like "uint16",
> "uint32", etc), which can be customized based on your compiler.  Then,
> instead of using types like "unsigned long" and "unsigned int" throughout
> the code (when the code really means: 32bits or 16 bits), use the new
> "uint32/uint16" types.

Sounds good, IMHO this would be the right approach.

Bye
Alex
-- 
Work: ####@####.#### - http://www.jenoptik-los.de
Home: ####@####.####                - http://www.kde.org
      ####@####.####               - http://www.neundorf.net

 > I suggest a global header file with 'generic' typedefs (like "uint16",
 > "uint32", etc), which can be customized based on your compiler.  Then,
 > instead of using types like "unsigned long" and "unsigned int"
 > throughout the code (when the code really means: 32bits or 16 bits), use
 > the new "uint32/uint16" types.

You need to be extremely careful - I cannot emphasize that enough.

Remember that a good chunk of the 'unsigned longs' you find in the 
Microwindows code are true pointers, and should be kept in the native 
long format of the processor. Really, the only places where you would 
specifically need a 32 bit type would be in the image and font decoding 
sections, and a little bit in the drawing code where we probably make 
some silly assumptions about the size of a buffer.  The rest of the code 
is pretty much designed only to work with itself on the target system 
anyway.   I think it would be far more damaging to have to figure out 
why a pointer that was turned into a uint32 by accident deep in the 
engine suddenly doesn't work on an amd64 machine then determining that 
your font doesn't look right because of a misplaced unsigned long in the 
  PCF engine.

If it was me, I would skip the global header files, and just use the 
common char/short/int designators for 8/16/32 and long for native 
pointers, but then again, I only use gcc and I'm lazy like that.  I 
guess that <sys/types.h> can help me out in a pinch, but I'll leave it 
to the rest of the group if a wholesale substitution of of 
char/short/int types is in order.

Jordan

>You need to be extremely careful - I cannot emphasize that enough.
>
>Remember that a good chunk of the 'unsigned longs' you find in the 
>Microwindows code are true pointers, and should be kept in the native long 
>format of the processor.

Of course.  As I said:

>Then, instead of using types like "unsigned long" and "unsigned int"
>throughout the code (when the code really means: 32bits or 16 bits), use
>the new "uint32/uint16" types.

The key phrase being "when the code really means: 32bits or 16 bits".  I do 
NOT mean that someone should use something like "sed" and replace every 
single "unsigned long" with "uint32".  It needs to be done with knowledge of 
the context of the code.  That's why I said it would probably be better for 
someone more familiar with the code to do it (rather than me).

>I think it would be far more damaging to have to figure out why a pointer 
>that was turned into a uint32 by accident deep in the engine suddenly 
>doesn't work on an amd64 machine then determining that your font doesn't 
>look right because of a misplaced unsigned long in the  PCF engine.

Again, we wouldn't be changing 'pointers' to 'uint32' - only 'unsigned 
longs' that are *meant to be* '32bit integer'.

But, "determining that your font doesn't look right" doesn't seem the right 
way to do it either.  There could be tons of cases of mis-used types where 
(for example) memory is being corrupted (on certain platforms, like where 
"unsigned long" != 32bits).  I ran into the font issue immediately, 
*because* it's "visible".  It doesn't mean that other parts of the code 
aren't doing the wrong thing on my platform - just because there wasn't an 
immediatly "visible" problem on the screen.


So, to re-state (and perhaps change things to be a bit less "scary"): I 
suggest a global header file with these special typedefs.  If you are 
working on a module that needs to address things as specific bit sizes (like 
32bits), then change things like "unsigned long" to "uint32".  Because I've 
spent some time in "font_pcf.c", I'd feel comfortable doing it there.  Other 
people that are comfortable with other modules could (if they feel inclined 
to make the code more 'portable') modify those modules where applicable.

That's my suggestion.  Overall, I'm pleasantly surprised at how 'portable' 
Microwindows/Nano-X have been, but there still seem to be some portability 
issues (like this problem, and the 'endian' issues I've also run into).  I 
assume it's a goal of the project to be very portable - this can only help, 
IMO.

Regards,

- Paul B.

Alexander Neundorf wrote:
> On Thursday 30 June 2005 21:44, Paul Bartholomew wrote:
> ...
> > I suggest a global header file with 'generic' typedefs (like "uint16",
> > "uint32", etc), which can be customized based on your compiler.  Then,
> > instead of using types like "unsigned long" and "unsigned int"
throughout
> > the code (when the code really means: 32bits or 16 bits), use the new
> > "uint32/uint16" types.
>
> Sounds good, IMHO this would be the right approach.

GCC and many other compilers do support this:

  #include <stdbool.h>
  #include <stdint.h>

  bool x = true, y = false;
  uint8_t a;
  uint16_t b;
  uint32_t c;
  uint64_t d;
  int8_t e;
  int16_t f;
  int32_t g;
  int64_t h;

this is a widespread and standarized C convention.
platforms that dont support these convention
wont take much user effort for the missing header.

for the sake of pointers, as user and system interpretation
might be different depending on misc factors like given flags,
the most generic approch is this:

  void some_api_function (void *buffer, int flags);

this saves the user a bunch of typecasts
and enforces the system coder (unless the compiler is crap)
to explicitely apply a typcast on the buffer for the desired
data width which will impact any sort of pointer increment.

entry points with a fixed data width should not need the void-style.

-Alex.

On Fri, Jul 01, 2005 at 10:25:54AM +0200, Alexander Stohr wrote:

> Alexander Neundorf wrote:
> > On Thursday 30 June 2005 21:44, Paul Bartholomew wrote:
> > ...
> > > I suggest a global header file with 'generic' typedefs (like
> > > "uint16", "uint32", etc), which can be customized based on your
> > > compiler.  Then, instead of using types like "unsigned long" and
> > > "unsigned int" throughout the code (when the code really means:
> > > 32bits or 16 bits), use the new "uint32/uint16" types.
> >
> > Sounds good, IMHO this would be the right approach.
> 
> GCC and many other compilers do support this:
> 
>   #include <stdbool.h>
>   #include <stdint.h>
> 
>   bool x = true, y = false;
>   uint8_t a;
>   uint16_t b;
>   uint32_t c;
>   uint64_t d;
>   int8_t e;
>   int16_t f;
>   int32_t g;
>   int64_t h;

I believe these are part of C99 standard:
http://www.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html
http://www.opengroup.org/onlinepubs/009695399/basedefs/stdbool.h.html

use 'em when # of bits is meaningful.

-- 
  Aaron J. Grier  |   Frye Electronics, Tigard, OR   |  ####@####.####

: > I suggest a global header file with 'generic' typedefs (like "uint16",
:  > "uint32", etc), which can be customized based on your compiler.  Then,
:  > instead of using types like "unsigned long" and "unsigned int"
:  > throughout the code (when the code really means: 32bits or 16 bits),
use
:  > the new "uint32/uint16" types.
:
: You need to be extremely careful - I cannot emphasize that enough.

I tend to agree with Jordan on this.  We can get into trouble
when we start using uint32's without completely understanding
the full ramifications.  Having said that, Microwindows is
basically too big these days to run on 16 bit systems.

: Overall, I'm pleasantly surprised at how 'portable'
Microwindows/Nano-X have been, but there still seem to be some portability
issues (like this problem, and the 'endian' issues I've also run into).  I
assume it's a goal of the project to be very portable - this can only help,

Agreed.  I'm very big on portability and have thought about
this constantly during Microwindows development.  (see my
comments about const earlier).  One of the reasons things
work well now is that I've tried to use simple "int" and "long"
declarations, and only force sizes in image decoding.
This has worked well, because the move from 16 to 32 bit
compilers has been very well thought out.  It seems Paul's
issues are the result of moving to 64 bit systems, and I have
to admit, I'd imagined that unsigned longs would be 32 bits,
so there's likely places this needs to change.


I'd suggest first getting advice on where the problems are in the
engine and driver layers, and then afterwards deciding on the
best approach to more portability.

We also need to find some folks to volunteer testing big-endian;
I don't have such a system to test with, which is why the
big-endian bugs are there in the first place.

Regards,

Greg