Bug Hunting Session
Bug 75280 - Convert inappropriate use of sal_uIntPtr to better integer types
Summary: Convert inappropriate use of sal_uIntPtr to better integer types
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0 target:5.3.0 target:5.4....
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
: 91527 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-02-20 20:53 UTC by Michael Stahl (CIB)
Modified: 2019-04-12 08:43 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
changes in unqidx.hxx (2.72 KB, text/x-csrc)
2015-03-08 12:09 UTC, mridul
Details
sal_uintptr change for debug.hxx (3.82 KB, text/x-csrc)
2015-03-08 14:22 UTC, mridul
Details
zcodec.hxx fix (2.99 KB, text/x-csrc)
2015-03-08 14:33 UTC, mridul
Details
inetstrm.hxx (5.69 KB, text/plain)
2015-03-08 14:34 UTC, mridul
Details
inetmsg.hxx (15.26 KB, text/plain)
2015-03-08 14:38 UTC, mridul
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Stahl (CIB) 2014-02-20 20:53:32 UTC
During the removal of the tools types, the ULONG type was initially
converted to sal_uIntPtr, before those doing that work switched to
using the stop-gap sal_uLong type instead.

sal_uIntPtr has a very special purpose, variables of this type
hold pointers that are converted to integers.

any use of the sal_uIntPtr type that is actually "just" an integer
should be replaced by an appropriate unsigned integer type,
depending on the situation sal_uInt32, sal_uInt64, size_t or
"unsigned int" are likely candidates.

as an example, take the UniqueIndexImpl class.
it stores sal_uInt32 internally but its public interfaces are
defined on sal_uIntPtr; it is obvious that sal_uIntPtr should
be replaced by sal_uInt32 here in UniqueIndexImpl and its clients.
Comment 1 Alexandre Vicenzi 2014-02-21 11:09:19 UTC
As I'm working in 63154, I will take a look in this bug.
Comment 2 Commit Notification 2014-03-28 19:14:01 UTC
Valentin Kettner committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=061130bd6d60c648991afb83bc0befb48a68f34e

fdo#75280 Started cleaning up of sal_uIntPtr usage.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 3 Michael Stahl (CIB) 2014-12-02 13:03:37 UTC
looks like Alexandre isn't actually working on this; un-assigning...

actually the clean-up here should parallelize easily so multiple people can work on it anyway.
Comment 4 ababaaa 2015-02-13 17:28:45 UTC
I would like to work on this bug.Does this bug simply means to convert ALL the sal_uIntPtr to approtiate types like  sal_uInt32, sal_uInt64.
Comment 5 Stephan Bergmann 2015-02-16 08:43:58 UTC
(In reply to Lovekesh Garg from comment #4)
> I would like to work on this bug.Does this bug simply means to convert ALL
> the sal_uIntPtr to approtiate types like  sal_uInt32, sal_uInt64.

no; sal_uIntPtr should be used exactly in those places where its semantics are asked for---an (unsigned) integer large enough to hold a pointer
Comment 6 mridul 2015-03-08 12:09:46 UTC
Created attachment 113969 [details]
changes in unqidx.hxx
Comment 7 mridul 2015-03-08 12:11:26 UTC
please someone have a look at the attachment and let me know if its correct or not..thank you
Comment 8 mridul 2015-03-08 14:22:44 UTC
Created attachment 113970 [details]
sal_uintptr change for debug.hxx
Comment 9 mridul 2015-03-08 14:33:00 UTC
Created attachment 113971 [details]
zcodec.hxx fix
Comment 10 mridul 2015-03-08 14:34:24 UTC
Created attachment 113972 [details]
inetstrm.hxx
Comment 11 mridul 2015-03-08 14:38:30 UTC
Created attachment 113973 [details]
inetmsg.hxx
Comment 12 David Tardon 2015-03-08 15:43:12 UTC
dtardon->mridul: Please do not add complete source files as attachments. If you want to submit a patch, use gerrit.
Comment 13 Julien Nabet 2015-03-08 15:48:34 UTC
mridul: just for your information: https://wiki.documentfoundation.org/Development/gerrit
Comment 14 Huzaifa Iftikhar 2015-03-22 05:48:10 UTC
Hi,
I would like to work on this bug. Mark as assigned.
Comment 15 Thorsten Behrens (CIB) 2015-05-23 13:44:29 UTC
*** Bug 91527 has been marked as a duplicate of this bug. ***
Comment 16 Thorsten Behrens (CIB) 2015-05-23 13:52:21 UTC
To extract the relevant info from the duplicate bug into this one here - I think size_t would be a reasonably default type, given the fact the original type was ULONG.
Comment 17 Robinson Tryon (qubit) 2015-12-10 11:41:07 UTC Comment hidden (obsolete)
Comment 18 Robinson Tryon (qubit) 2016-02-18 14:52:20 UTC Comment hidden (obsolete)
Comment 19 Commit Notification 2016-03-08 22:26:50 UTC
Jaskaran committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=45701913f642b17aabd67b52de9002cc79cf07ae

tdf#75280 Replace sal_uIntPtr to better types in /connectivity

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 jani 2016-05-16 05:47:30 UTC
Changing status to reflect this bug is being worked on.
Comment 21 jani 2016-06-16 06:03:20 UTC Comment hidden (obsolete)
Comment 22 Commit Notification 2016-06-27 10:20:10 UTC
tymyjan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=08fc0da4033b8ea2b3ae67aa06175e839771396b

tdf#75280 Cleaning up of sal_uIntPtr usage #1a

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2016-07-09 07:53:51 UTC
tymyjan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5a6ab81651a98dd726ab7d40101dc81f62895fd4

tdf#75280 Cleaning up of sal_uIntPtr usage #2

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2016-07-11 07:31:49 UTC
tymyjan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e9bded5b87b491c0a0d7a62512e9c0923a08ce28

tdf#75280 Cleaning up of sal_uIntPtr usage #3

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2016-07-26 15:38:50 UTC
tymyjan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d2e4753c3f511cfc6b2932ce60d0bc2e09296f9f

tdf#75280 Cleaning up of sal_uIntPtr usage #4a

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Sudarshan K 2016-10-15 09:17:39 UTC
A general question,
1. How do I decide which one to replace sal_uIntPtr with ? That is, how do I decide to use 16/32/64 bit invariant of sal_uInt ? Any tests I should run etc ?
2. Also according to https://bugs.documentfoundation.org/show_bug.cgi?id=75280#c5, Comment 5, How do I decide where sal_uIntPtr and where sal_uInt variants are used ?

Cliffnotes: How to decide when to use sal_uInt and if it is to be used, which variant (16/32/64 etc) to be used ?

Thanks.
Comment 27 jani 2016-10-15 11:20:32 UTC
(In reply to Sudarshan K from comment #26)
> A general question,
> 1. How do I decide which one to replace sal_uIntPtr with ? That is, how do I
> decide to use 16/32/64 bit invariant of sal_uInt ? Any tests I should run
> etc ?
> 2. Also according to
> https://bugs.documentfoundation.org/show_bug.cgi?id=75280#c5, Comment 5, How
> do I decide where sal_uIntPtr and where sal_uInt variants are used ?
that is the challenge in this easyhack, read the first comment it explains it quite good.

also take a look at some of the commits already done.
> 
> Cliffnotes: How to decide when to use sal_uInt and if it is to be used,
> which variant (16/32/64 etc) to be used ?
look at the usage of the variable, some places are easy others are quite complicated.


> 

> Thanks.
Comment 28 Commit Notification 2016-10-24 15:50:08 UTC
Hieronymous committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=39851c7f780e548d494a9ef8bdf256b32a631f00

tdf#75280 Clean up usage of sal_uIntPtr.

It will be available in 5.3.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2016-12-07 15:57:20 UTC
Gaurav Dhingra committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c52745f8a3743841a4de030928c61d22063dd8ec

tdf#75280 Convert few inappropriate use of sal_uIntPtr to better integer types

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Commit Notification 2017-04-25 08:59:34 UTC
Chris Sherlock committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=437a5d7c72547f6f17b4ffecd51e76a2487de99e

tdf#75280: convert sal_uIntPtr to sal_uInt32 for ErrorInfo member

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2017-05-12 17:36:23 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=05d3a1899eb50202fd3929b702bae1003b5610be

tdf#75280 replace uses of sal_uLong

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 32 Commit Notification 2019-04-12 08:43:06 UTC
Regis committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/70040ba199ab34a792beb34cbafdbc8edc0e22ea%5E%21

tdf#75280 -Convert inappropriate use of sal_uIntPtr to better integ types

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.