View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000526 | LDMud 3.3 | Portability | public | 2008-01-26 15:10 | 2022-10-06 22:00 |
| Reporter | zesstra | Assigned To | |||
| Priority | normal | Severity | feature | Reproducibility | always |
| Status | new | Resolution | open | ||
| Platform | Darwin 9.1.0 | OS | MacOS X | OS Version | 10.5.1 |
| Product Version | 3.3 | ||||
| Summary | 0000526: Implicit conversions of 64-bit values into 32-bit values | ||||
| Description | The 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 Reproduce | Compile the driver on MacOS X 10.5 with -Wshorten-64-to-32. | ||||
| Tags | No tags attached. | ||||
| Attached Files | |||||
|
|
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? |
|
|
> 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. |
|
|
> 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)? |
|
|
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.". |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2008-01-26 15:10 | zesstra | New Issue | |
| 2008-01-26 15:10 | zesstra | File Added: build.log | |
| 2008-01-27 15:48 | zesstra | Note Added: 0000590 | |
| 2008-01-27 16:06 | Largo | Note Added: 0000591 | |
| 2008-01-27 16:20 | Gnomi | Note Added: 0000592 | |
| 2008-01-30 15:01 | zesstra | Note Added: 0000594 | |
| 2008-07-16 16:28 | zesstra | Relationship added | child of 0000555 |
| 2022-10-06 22:00 | zesstra | Relationship deleted | child of 0000555 |
