View Issue Details

IDProjectCategoryView StatusLast Update
0000526LDMud 3.3Portabilitypublic2022-10-07 00:00
Reporterzesstra Assigned To 
PrioritynormalSeverityfeatureReproducibilityalways
Status newResolutionopen 
PlatformDarwin 9.1.0OSMacOS XOS Version10.5.1
Product Version3.3 
Summary0000526: Implicit conversions of 64-bit values into 32-bit values
DescriptionThe gcc in MacOS X 10.5 has a switch for enabling some interesting warnings. -Wshorten-64-to-32 warns about implicit conversions (truncations) of 64-bit values into 32-bit values. This is interesting for people who use the driver on 64-bit platforms, I guess, because this conversion is generally not lossless.
I learned about this switch today and enabled it to satisfy my curiosity. ;-)
I got 643 warning during building the driver.
I will attach the build log to this bug, because I guess, at sometime we have to look into these warning, as that truncations may well cause some bugs and maybe people working on 64-bit platforms want to have a look.
Steps To ReproduceCompile the driver on MacOS X 10.5 with -Wshorten-64-to-32.
TagsNo tags attached.

Activities

2008-01-26 16:10

 

build.log (81,666 bytes)

zesstra

2008-01-27 16:48

administrator   ~0000590

I had a quick glance at some of these warnings and identified some general cases until now. Perhaps we should think a moment, how to handle them consistently.
(I assume for now the 64-bit platform having the LP64 data model: longs and pointer 64 bits, ints 32 bit, shorts 16 bits.)

1. Using svalue.u.number or p_int/p_uint as input values, calculate something
   and assign the result to int.
   If we can be sure, that the result always fits into int, we could
   add an explicit cast to int. But maybe we should just use p_int?
2. assign svalue.u.number to an int value. I think, these cases should be
   changed to p_int val = svalue.u.number;
   Especially something like
   int d;
   if ( 0 != (d = (left->u.number >> 1) - (right->u.number >> 1)) )
   if ( 0 != (d = left->u.number - right->u.number) )
   suffers from this. (from i-svalue.cmp.h)
3. assigning time stamps (like current_time) to int. Here we should
   definitely use mp_int (current_time is mp_int) or IMHO better time_t
   for all time values.
4. svalue.x.generic: in svalue.h is mentioned, this generic is usually
   svalue.u.number << 1 and x.generic is also used as 'secondary type
   field' handle for comparisons, don't know the details here...
   Are there anywhere any details about x.generic? BTW: u.number is
   p_int (8 bytes) and x.generic is ph_int (2 bytes).
5. assigning return values from long-functions to int (e.g.
   find_function in closure.c). Should be changed, I think, probably
   by changing the assignment. I assume, the return values of functions
   are chosen deliberately. ;-)
6. Assigning pointers or differences of pointers to int, e.g.
   int ix = ... (current_variables - current_object->variables);
   where variables is a pointer to svalues. May be a problem or not, but
   I would suggest to avoid any risks... (This issue is similar to 1)
   BTW: What to use for pointers? p_int? Or intptr_t?

What do you think?

Largo

2008-01-27 17:06

reporter   ~0000591

> IMHO better time_t for all time values.
I strongly agree to this. time_t is not only the standard typedef for time values, it should also produce more readable code.

Gnomi

2008-01-27 17:20

manager   ~0000592

> IMHO better time_t for all time values.
I disagree. Finally all timestamps end up in the MUD as LPC values. So at one point it has to be converted to p_int (the type of svalue.u.number), so why not do it at the source (current_time)?

zesstra

2008-01-30 16:01

administrator   ~0000594

Im not so sure. Right now, time_t and p_int are both typedefed to long, so there is no conversion necessary.
If that should change, I see basically 2 possibilities:
a) in a way that an impplicit conversion is lossless
b) if p_int and time_t are of 2 incompatible types we would have problems
   in any case: Either the conversion to svalues is not lossless and we
   get garbage from efuns which deal with time or the driver uses an
   incompatible data type for calling the libc functions which deal with
   time values, which can also be very ugly.

I personally agree, that time_t is more readable.
BTW: from backend.c I noticed concerning current_time: "TODO: Should be time_t if it is given that time_t is always a skalar.".

Issue History

Date Modified Username Field Change
2008-01-26 16:10 zesstra New Issue
2008-01-26 16:10 zesstra File Added: build.log
2008-01-27 16:48 zesstra Note Added: 0000590
2008-01-27 17:06 Largo Note Added: 0000591
2008-01-27 17:20 Gnomi Note Added: 0000592
2008-01-30 16:01 zesstra Note Added: 0000594
2008-07-16 18:28 zesstra Relationship added child of 0000555
2022-10-07 00:00 zesstra Relationship deleted child of 0000555