Issue 66726 - m173: casting pointer to unsigned int in tools/source/inet/inetmsg.cxx
Summary: m173: casting pointer to unsigned int in tools/source/inet/inetmsg.cxx
Status: CLOSED FIXED
Alias: None
Product: porting
Classification: Code
Component: code (show other issues)
Version: current
Hardware: PC (x86_64) Linux, all
: P3 Trivial (vote)
Target Milestone: OOo 2.0.4
Assignee: kendy
QA Contact: issues@porting
URL:
Keywords:
Depends on:
Blocks: 66757
  Show dependency tree
 
Reported: 2006-06-25 08:33 UTC by pavel
Modified: 2007-05-29 20:49 UTC (History)
3 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
The patch. (846 bytes, patch)
2006-06-26 14:51 UTC, kendy
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description pavel 2006-06-25 08:33:29 UTC
Hi,

in m173 with warnings turned into errors:

/data/oo/BuildDir/ooo_SRC680_m173_src/tools/source/inet/inetmsg.cxx: In member
function 'BOOL INetMIMEMessage::EnableAttachChild(INetMessageContainerType)':
/data/oo/BuildDir/ooo_SRC680_m173_src/tools/source/inet/inetmsg.cxx:1395: error:
cast from 'INetMIMEMessage*' to 'unsigned int' loses precision
dmake:  Error code 1, while making '../../unxlngx6.pro/obj/inetmsg.obj'

The code there is:

        // Generate a unique boundary from current time.
        sal_Char sTail[16 + 1];
        Time aCurTime;
        sprintf (sTail, "%08X%08X",
                 static_cast< unsigned int >(aCurTime.GetTime()),
                 reinterpret_cast< unsigned int >(this));
Comment 1 kendy 2006-06-26 14:50:31 UTC
sb: Would be something like the following OK for you, please?  If yes - OK to 
commit to warningfixes01?
Comment 2 kendy 2006-06-26 14:51:31 UTC
Created attachment 37327 [details]
The patch.
Comment 3 kendy 2006-06-26 14:57:31 UTC
patch, targetted 2.0.4
Comment 4 Stephan Bergmann 2006-06-26 15:39:29 UTC
The proposed patch would lead to warnings from GCC 4.1.1 about mixing "%X" with
"sal_uInt32".  Thus, I would suggest to additionally change "%08X%08X" to
"%08lX%08lX" and change the two static_cast<sal_uInt32> to static_cast<unsigned
long>.

warningfixes01 is ok (warningfixes01 was opened on SRC680m172, cd will resync it
to SRC680m173 as soon as that version is available (tomorrow))
Comment 5 kendy 2006-06-26 16:03:30 UTC
Oh, right, I see - sal_uInt32 is defined as unsigned long on 32bit.

I would go with static_cast< unsigned int >(aCurTime.GetTime()) and 
static_cast< unsigned int >(nThis) then; rather than %lX...  Would that be OK 
for you, please?
Comment 6 kendy 2006-06-26 16:09:06 UTC
When we are at it (sorry for opening it in this issue ;-) ), wouldn't it be 
worth changing the order of

#if SAL_TYPES_SIZEOFLONG == 4
	typedef signed long       sal_Int32;
	typedef unsigned long     sal_uInt32;
#elif SAL_TYPES_SIZEOFINT == 4
	typedef signed int        sal_Int32;
	typedef unsigned int      sal_uInt32;
#else
     #error "Could not find 32-bit type, add support for your architecture"
#endif

in sal/inc/sal/types.h to check for SAL_TYPES_SIZEOFINT first - so that 
sal_u?Int32 is mostly (unsigned|) int? ;-)
Comment 7 Stephan Bergmann 2006-06-26 16:25:59 UTC
@kendy:

- What is it that you do not like about the %lX/unsigned long version?   It IMO
is the most portable solution, as unsigned long is guaranteed to have >= 32bit,
while unsigned int is not (granted, somewhat theoretical point).

- I fear that changing sal/inc/sal/types.h would be incompatible.
Comment 8 kendy 2006-06-26 17:06:05 UTC
1) The >= 32 was the problem.  On 64bit:

kendy@one:~> cat blah.cc
#include <cstdio>

int main( int, char ** )
{
    unsigned long l = 0x1234567887654321;
    printf( "%08lX\n", l );
    return 0;
}
kendy@one:~> g++ blah.cc -o blah
kendy@one:~> ./blah
1234567887654321

Of course, nothing that couldn't be addressed ;-) - but I wanted to keep the 
patch minimal.

2) Yes, the methods would be mangled differently after that change...  But on 
the other hand, 64bit OOo uses 'int' for sal_u?Int32 - so then the 32bit & 
64bit versions would be more alike ;-)
Comment 9 Stephan Bergmann 2006-06-27 08:30:48 UTC
1)  Yes, I see, assuming that the intent of the code is to emit exactly eight
digits, your patch is the way to go.
Comment 10 kendy 2006-06-27 12:56:52 UTC
Committed to CWS warningfixes01 [just 1), of course ;-) ].
Comment 11 kendy 2006-06-29 14:15:40 UTC
VERIFIED, seen is warningfixes01.
Comment 12 caolanm 2007-05-29 20:49:47 UTC
closing