569611 - hunspell compilation failure on mingw-w64 due to pointer to long cast loosing precision.

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

I get following errors:

extensions/spellcheck/hunspell/src/csutil.cpp:290:40: error: cast from 'char*' to 'long unsigned int' loses precision
extensions/spellcheck/hunspell/src/csutil.cpp:290:60: error: cast from 'char*' to 'long unsigned int' loses precision
extensions/spellcheck/hunspell/src/dictmgr.cpp:179:39: error: cast from 'char*' to 'long unsigned int' loses precision
extensions/spellcheck/hunspell/src/dictmgr.cpp:179:59: error: cast from 'char*' to 'long unsigned int' loses precision

These casts are used for char pointer subtraction. They are not needed at all as char is of 1 byte size, subtracting pointers will give expected value.

Comment 1

[Caolan McNamara]

Yeah, fix checked in upstream for hunspell 1.2.12

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 448771 [details] [diff] [review]
fix v1.0

       if (dp) {
          *stringp = dp+1;
-         int nc = (int)((unsigned long)dp - (unsigned long)mp);
+         int nc = dp - mp;

Are there any guarantees that dp - mp fits in a 32-bit int here? Seems like nc should a size_t here to be sure, or explicit length checks need to ensure that this will never overflow.

Comment 3

[Jacek Caban]

Attachment: fix v1.1

It's not really the problem my patch is solving, but the attached version uses size_t. If we're going to take it into account, the result of strlen should be stored in size_t as well. Also one of pointer subtractions isn't needed at all as we already have the pointer we want.

Comment 4

[Caolan McNamara]

Yeah, its size_t upstream for these ones, should probably be size_t more generally in more places, but that's not the exact problem mentioned here which was getting it to compile in the first place :-)

Comment 5

[Jacek Caban]

Attachment: fix v1.2 (rebased)

The patch rebased against current m-c.

Comment 6

[Jacek Caban]

Caolan, thanks for mainstreaming it. What's the next step? Can this patch be checked into m-c or should we wait for the next merge? If we should wait, is there any ETA for merge?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

My plan was to wait for the next release to get it into the tree.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

This is fixed now that 1.2.12 has landed on trunk.