Drizzle

Merge lp:~skinny.moey/drizzle/drizzle-warnings-conversion into lp:~drizzle-trunk/drizzle/development

  1. drizzle-warnings-conversion
  2. Merge into development
Proposed by Joe Daly
Status: Rejected
Rejected by: Joe Daly
Proposed branch: lp:~skinny.moey/drizzle/drizzle-warnings-conversion
Merge into: lp:~drizzle-trunk/drizzle/development
Diff against target: 476 lines
6 files modified
mystrings/ctype-bin.cc (+8/-7)
mystrings/ctype-mb.cc (+9/-7)
mystrings/ctype-simple.cc (+31/-28)
mystrings/ctype-uca.cc (+15/-12)
mysys/ptr_cmp.cc (+5/-5)
mysys/tree.cc (+10/-5)
To merge this branch: bzr merge lp:~skinny.moey/drizzle/drizzle-warnings-conversion
Reviewer Review Type Date Requested Status
Drizzle Developers Pending
Review via email: mp+12780@code.launchpad.net

This proposal supersedes a proposal from 2009-09-25.

To post a comment you must log in.
Revision history for this message
Joe Daly (skinny.moey) wrote : Posted in a previous version of this proposal

Changes to eventually enable -Wconversion to be turned on.

Revision history for this message
Jay Pipes (jaypipes) wrote : Posted in a previous version of this proposal

Well, Joe, you've picked some of the ugliest code to dig into! :)

I'm going to take some extra time and review this carefully. There are a couple things I'm concerned about and want to double-check with my C++ standards books to verify.

Namely:

1) There are a couple places where I see uint32_t being used where I believe ptr_diff_t is appropriate.

2) static_cast<short unsigned int>() should be static_cast<uint16_t>() ?

3) static_cast<unsigned long>() should be static_cast<uint32_t>() ?

Anyway, I just want to make sure, so there may be a bit of a delay on this, ok?

Thanks for your patience!

Jay

Revision history for this message
Stewart Smith (stewart) wrote : Posted in a previous version of this proposal

What about enabling -Wconversion as we go through places that can be built with it? e.g. I guess after this patch we could do it for mysys/ ?

Revision history for this message
Joe Daly (skinny.moey) wrote : Posted in a previous version of this proposal

> What about enabling -Wconversion as we go through places that can be built
> with it? e.g. I guess after this patch we could do it for mysys/ ?

Good idea! I should be able enable it for mystrings and mysys in my next commit.

Revision history for this message
Joe Daly (skinny.moey) wrote :

I changed short unsigned int -> uint16_t and unsigned long to uint32_t.

I could use a little bit of direction on where I should use ptrdiff_t after some reading about it and looking at the changes I didnt see where it should be used.

Revision history for this message
Brian Aker (brianaker) wrote :

> I changed short unsigned int -> uint16_t and unsigned long to uint32_t.
>
> I could use a little bit of direction on where I should use ptrdiff_t after
> some reading about it and looking at the changes I didnt see where it should
> be used.

An int should be a uint32_t and a long should go to a uint64_t (assuming all logic lined up).

Looking at st_tree, allocated should have been a uint64_t.

Revision history for this message
Joe Daly (skinny.moey) wrote :

moved to rejected state, needs additional work on what to do with reinterpret casts to unsigned char*

Unmerged revisions

1140. By Joe Daly

change short unsigned int to uint16_t

1139. By Joe Daly

change unsigned long to uint32_t

1138. By Joe Daly

style change

1137. By Joe Daly

update with trunk

1136. By Joe Daly

Changes to allow compilation with -Wconversion turned on. Also change iteration type to uint16_t from uint32_t this was producing a conversion warning, there is no need for a conversion, rather change the index type.

1135. By Joe Daly

merge with trunk

1134. By Joe Daly

changes to allow -Wconversiion to be turned on

1133. By Joe Daly

changes to allow -Wconversiion to be turned on

1132. By Joe Daly

changes to allow -Wconversiion to be turned on

1131. By Joe Daly

changes to allow -Wconversiion to be turned on

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'mystrings/ctype-bin.cc'
2--- mystrings/ctype-bin.cc 2009-07-07 09:06:29 +0000
3+++ mystrings/ctype-bin.cc 2009-10-02 12:35:19 +0000
4@@ -284,8 +284,8 @@
5
6 for (; pos < (unsigned char*) key ; pos++)
7 {
8- nr1[0]^=(ulong) ((((uint32_t) nr1[0] & 63)+nr2[0]) *
9- ((uint32_t)*pos)) + (nr1[0] << 8);
10+ nr1[0]^= ((nr1[0] & 63) + nr2[0]) * (static_cast<uint32_t>(*pos)) +
11+ (nr1[0] << 8);
12 nr2[0]+=3;
13 }
14 }
15@@ -301,8 +301,8 @@
16
17 for (; pos < (unsigned char*) key ; pos++)
18 {
19- nr1[0]^=(ulong) ((((uint32_t) nr1[0] & 63)+nr2[0]) *
20- ((uint32_t)*pos)) + (nr1[0] << 8);
21+ nr1[0]^= ((nr1[0] & 63) + nr2[0]) * (static_cast<uint32_t>(*pos)) +
22+ (nr1[0] << 8);
23 nr2[0]+=3;
24 }
25 }
26@@ -405,7 +405,7 @@
27 if (dst != src)
28 memcpy(dst, src, srclen);
29 return my_strxfrm_pad_desc_and_reverse(cs, dst, dst + srclen, dst + dstlen,
30- nweights - srclen, flags, 0);
31+ static_cast<uint32_t>(nweights - srclen), flags, 0);
32 }
33
34
35@@ -451,13 +451,14 @@
36 if (nmatch > 0)
37 {
38 match[0].beg= 0;
39- match[0].end= (size_t) (str- (const unsigned char*)b-1);
40+ match[0].end= static_cast<uint32_t>((str -
41+ reinterpret_cast<const unsigned char*>(b) - 1));
42 match[0].mb_len= match[0].end;
43
44 if (nmatch > 1)
45 {
46 match[1].beg= match[0].end;
47- match[1].end= match[0].end+s_length;
48+ match[1].end= static_cast<uint32_t>(match[0].end + s_length);
49 match[1].mb_len= match[1].end-match[1].beg;
50 }
51 }
52
53=== modified file 'mystrings/ctype-mb.cc'
54--- mystrings/ctype-mb.cc 2009-07-09 00:04:44 +0000
55+++ mystrings/ctype-mb.cc 2009-10-02 12:35:19 +0000
56@@ -348,12 +348,12 @@
57 if (nmatch)
58 {
59 match[0].beg= 0;
60- match[0].end= (size_t) (b-b0);
61+ match[0].end= static_cast<uint32_t>(b - b0);
62 match[0].mb_len= res;
63 if (nmatch > 1)
64 {
65 match[1].beg= match[0].end;
66- match[1].end= match[0].end+s_length;
67+ match[1].end= static_cast<uint32_t>(match[0].end + s_length);
68 match[1].mb_len= 0; /* Not computed */
69 }
70 }
71@@ -582,8 +582,8 @@
72
73 for (; pos < (const unsigned char*) key ; pos++)
74 {
75- nr1[0]^=(ulong) ((((uint32_t) nr1[0] & 63)+nr2[0]) *
76- ((uint32_t)*pos)) + (nr1[0] << 8);
77+ nr1[0]^= ((nr1[0] & 63) + nr2[0]) * (static_cast<uint32_t>(*pos)) +
78+ (nr1[0] << 8);
79 nr2[0]+=3;
80 }
81 }
82@@ -616,8 +616,10 @@
83 return;
84 }
85
86- buflen= cs->cset->wc_mb(cs, cs->max_sort_char, (unsigned char*) buf,
87- (unsigned char*) buf + sizeof(buf));
88+ buflen= static_cast<char>(cs->cset->wc_mb(cs, cs->max_sort_char,
89+ reinterpret_cast<unsigned char*>(buf),
90+ reinterpret_cast<unsigned char*>(buf)
91+ + sizeof(buf)));
92
93 assert(buflen > 0);
94 do
95@@ -1126,7 +1128,7 @@
96 continue;
97 }
98 b+= mb_len;
99- pg= (wc >> 8) & 0xFF;
100+ pg= static_cast<uint32_t>((wc >> 8) & 0xFF);
101 clen+= utr11_data[pg].p ? utr11_data[pg].p[wc & 0xFF] : utr11_data[pg].page;
102 clen++;
103 }
104
105=== modified file 'mystrings/ctype-simple.cc'
106--- mystrings/ctype-simple.cc 2009-06-24 22:50:51 +0000
107+++ mystrings/ctype-simple.cc 2009-10-02 12:35:19 +0000
108@@ -84,7 +84,7 @@
109 unsigned char *d0= dst;
110 uint32_t frmlen;
111 if ((frmlen= min((uint32_t)dstlen, nweights)) > srclen)
112- frmlen= srclen;
113+ frmlen= static_cast<uint32_t>(srclen);
114 if (dst != src)
115 {
116 const unsigned char *end;
117@@ -318,8 +318,9 @@
118
119 for (; key < end ; key++)
120 {
121- nr1[0]^=(ulong) ((((uint32_t) nr1[0] & 63)+nr2[0]) *
122- ((uint32_t) sort_order[(uint32_t) *key])) + (nr1[0] << 8);
123+ nr1[0]^= ((nr1[0] & 63) + nr2[0]) *
124+ (static_cast<uint32_t>(sort_order[static_cast<uint32_t>(*key)])) +
125+ (nr1[0] << 8);
126 nr2[0]+=3;
127 }
128 }
129@@ -400,11 +401,11 @@
130 for (c = *s; s != e; c = *++s)
131 {
132 if (c>='0' && c<='9')
133- c -= '0';
134+ c= static_cast<unsigned char>(c - '0');
135 else if (c>='A' && c<='Z')
136- c = c - 'A' + 10;
137+ c= static_cast<unsigned char>(c - 'A' + 10);
138 else if (c>='a' && c<='z')
139- c = c - 'a' + 10;
140+ c= static_cast<unsigned char>(c - 'a' + 10);
141 else
142 break;
143 if (c >= base)
144@@ -522,11 +523,11 @@
145 for (c = *s; s != e; c = *++s)
146 {
147 if (c>='0' && c<='9')
148- c -= '0';
149+ c= static_cast<unsigned char>(c - '0');
150 else if (c>='A' && c<='Z')
151- c = c - 'A' + 10;
152+ c= static_cast<unsigned char>(c - 'A' + 10);
153 else if (c>='a' && c<='z')
154- c = c - 'a' + 10;
155+ c= static_cast<unsigned char>(c - 'a' + 10);
156 else
157 break;
158 if (c >= base)
159@@ -637,11 +638,11 @@
160 {
161 register unsigned char c= *s;
162 if (c>='0' && c<='9')
163- c -= '0';
164+ c= static_cast<unsigned char>(c - '0');
165 else if (c>='A' && c<='Z')
166- c = c - 'A' + 10;
167+ c= static_cast<unsigned char>(c - 'A' + 10);
168 else if (c>='a' && c<='z')
169- c = c - 'a' + 10;
170+ c= static_cast<unsigned char>(c - 'a' + 10);
171 else
172 break;
173 if (c >= base)
174@@ -761,11 +762,11 @@
175 register unsigned char c= *s;
176
177 if (c>='0' && c<='9')
178- c -= '0';
179+ c= static_cast<unsigned char>(c - '0');
180 else if (c>='A' && c<='Z')
181- c = c - 'A' + 10;
182+ c= static_cast<unsigned char>(c - 'A' + 10);
183 else if (c>='a' && c<='z')
184- c = c - 'a' + 10;
185+ c= static_cast<unsigned char>(c - 'a' + 10);
186 else
187 break;
188 if (c >= base)
189@@ -866,13 +867,14 @@
190 }
191
192 new_val = (long) (uval / 10);
193- *--p = '0'+ (char) (uval - (unsigned long) new_val * 10);
194+ *--p= static_cast<char>('0'+ static_cast<char>(uval -
195+ static_cast<uint32_t>(new_val) * 10));
196 val = new_val;
197
198 while (val != 0)
199 {
200 new_val=val/10;
201- *--p = '0' + (char) (val-new_val*10);
202+ *--p= static_cast<char>('0' + static_cast<char>(val-new_val*10));
203 val= new_val;
204 }
205
206@@ -918,7 +920,7 @@
207 {
208 uint64_t quo= uval/(uint32_t) 10;
209 uint32_t rem= (uint32_t) (uval- quo* (uint32_t) 10);
210- *--p = '0' + rem;
211+ *--p= static_cast<char>('0' + rem);
212 uval= quo;
213 }
214
215@@ -1215,13 +1217,14 @@
216 if (nmatch > 0)
217 {
218 match[0].beg= 0;
219- match[0].end= (size_t) (str- (const unsigned char*)b-1);
220+ match[0].end= static_cast<uint32_t>(str -
221+ reinterpret_cast<const unsigned char*>(b) - 1);
222 match[0].mb_len= match[0].end;
223
224 if (nmatch > 1)
225 {
226 match[1].beg= match[0].end;
227- match[1].end= match[0].end+s_length;
228+ match[1].end= match[0].end + static_cast<uint32_t>(s_length);
229 match[1].mb_len= match[1].end-match[1].beg;
230 }
231 }
232@@ -1315,7 +1318,7 @@
233 if (wc >= idx[i].uidx.from && wc <= idx[i].uidx.to && wc)
234 {
235 int ofs= wc - idx[i].uidx.from;
236- idx[i].uidx.tab[ofs]= ch;
237+ idx[i].uidx.tab[ofs]= static_cast<unsigned char>(ch);
238 }
239 }
240 }
241@@ -1344,7 +1347,7 @@
242 static void set_max_sort_char(CHARSET_INFO *cs)
243 {
244 unsigned char max_char;
245- uint32_t i;
246+ uint16_t i;
247
248 if (!cs->sort_order)
249 return;
250@@ -1529,7 +1532,7 @@
251 }
252 }
253
254- digits= str - beg;
255+ digits= static_cast<int>(str - beg);
256
257 /* Continue to accumulate into uint64_t */
258 for (dot= NULL, ull= ul; str < end; str++)
259@@ -1566,7 +1569,7 @@
260 }
261 else
262 {
263- shift= dot - str;
264+ shift= static_cast<int>(dot - str);
265 for ( ; str < end && (ch= (unsigned char) (*str - '0')) < 10; str++) {}
266 }
267 goto exp;
268@@ -1590,7 +1593,7 @@
269 /* Unknown character, exit the loop */
270 break;
271 }
272- shift= dot ? dot - str : 0; /* Right shift */
273+ shift= dot ? static_cast<int>(dot - str) : 0; /* Right shift */
274 addon= 0;
275
276 exp: /* [ E [ <sign> ] <unsigned integer> ] */
277@@ -1865,14 +1868,14 @@
278 for (strend--; str <= strend;)
279 {
280 unsigned char tmp= *str;
281- *str++= ~*strend;
282- *strend--= ~tmp;
283+ *str++= static_cast<unsigned char>(~*strend);
284+ *strend--= static_cast<unsigned char>(~tmp);
285 }
286 }
287 else
288 {
289 for (; str < strend; str++)
290- *str= ~*str;
291+ *str= static_cast<unsigned char>(~*str);
292 }
293 }
294 else if (flags & (MY_STRXFRM_REVERSE_LEVEL1 << level))
295
296=== modified file 'mystrings/ctype-uca.cc'
297--- mystrings/ctype-uca.cc 2009-07-09 00:46:46 +0000
298+++ mystrings/ctype-uca.cc 2009-10-02 12:35:19 +0000
299@@ -6828,8 +6828,8 @@
300 }
301 else
302 {
303- scanner->page= wc >> 8;
304- scanner->code= wc & 0xFF;
305+ scanner->page= static_cast<int>(wc >> 8);
306+ scanner->code= static_cast<int>(wc & 0xFF);
307 }
308
309 if (scanner->contractions && !scanner->page &&
310@@ -6840,8 +6840,8 @@
311 if (((mb_len= scanner->cs->cset->mb_wc(scanner->cs, &wc,
312 scanner->sbeg,
313 scanner->send)) >=0) &&
314- (!(page1= (wc >> 8))) &&
315- ((code1= (wc & 0xFF)) > 0x40) &&
316+ (!(page1= static_cast<uint32_t>(wc >> 8))) &&
317+ ((code1= static_cast<uint32_t>(wc & 0xFF)) > 0x40) &&
318 (code1 < 0x80) &&
319 (cweight= scanner->contractions[(scanner->code-0x40)*0x40 + code1-0x40]))
320 {
321@@ -6862,7 +6862,7 @@
322 implicit:
323
324 scanner->code= (scanner->page << 8) + scanner->code;
325- scanner->implicit[0]= (scanner->code & 0x7FFF) | 0x8000;
326+ scanner->implicit[0]= static_cast<uint16_t>((scanner->code & 0x7FFF) | 0x8000);
327 scanner->implicit[1]= 0;
328 scanner->wbeg= scanner->implicit;
329
330@@ -7141,8 +7141,8 @@
331 for (; dst < de && nweights &&
332 (s_res= scanner_handler->next(&scanner)) > 0 ; nweights--)
333 {
334- *dst++= s_res >> 8;
335- *dst++= s_res & 0xFF;
336+ *dst++= static_cast<unsigned char>(s_res >> 8);
337+ *dst++= static_cast<unsigned char>(s_res & 0xFF);
338 }
339
340 if (dst < de && nweights && (flags & MY_STRXFRM_PAD_WITH_SPACE))
341@@ -7151,8 +7151,8 @@
342 s_res= cs->sort_order_big[0][0x20 * cs->sort_order[0]];
343 for (; space_count ; space_count--)
344 {
345- *dst++= s_res >> 8;
346- *dst++= s_res & 0xFF;
347+ *dst++= static_cast<unsigned char>(s_res >> 8);
348+ *dst++= static_cast<unsigned char>(s_res & 0xFF);
349 }
350 }
351 my_strxfrm_desc_and_reverse(d0, dst, flags, 0);
352@@ -7769,7 +7769,8 @@
353 if (!newweights[pagec])
354 {
355 /* Alloc new page and copy the default UCA weights */
356- uint32_t size= 256*newlengths[pagec]*sizeof(uint16_t);
357+ uint32_t size= static_cast<uint32_t>(256 * newlengths[pagec] *
358+ sizeof(uint16_t));
359
360 if (!(newweights[pagec]= (uint16_t*) (*alloc)(size)))
361 return 1;
362@@ -7793,7 +7794,8 @@
363 defweights[pageb] + chb*deflengths[pageb],
364 deflengths[pageb]*sizeof(uint16_t));
365 /* Apply primary difference */
366- newweights[pagec][chc*newlengths[pagec]]+= rule[i].diff[0];
367+ newweights[pagec][chc*newlengths[pagec]]= static_cast<uint16_t>(
368+ newweights[pagec][chc*newlengths[pagec]] + rule[i].diff[0]);
369 }
370
371 /* Copy non-overwritten pages from the default UCA weights */
372@@ -7844,7 +7846,8 @@
373 offsc= (rule[i].curr[0]-0x40)*0x40+(rule[i].curr[1]-0x40);
374
375 /* Copy base weight applying primary difference */
376- cs->contractions[offsc]= offsb[0] + rule[i].diff[0];
377+ cs->contractions[offsc]= static_cast<uint16_t>(
378+ offsb[0] + rule[i].diff[0]);
379 /* Mark both letters as "is contraction part */
380 contraction_flags[rule[i].curr[0]]= 1;
381 contraction_flags[rule[i].curr[1]]= 1;
382
383=== modified file 'mysys/ptr_cmp.cc'
384--- mysys/ptr_cmp.cc 2009-04-26 16:53:32 +0000
385+++ mysys/ptr_cmp.cc 2009-10-02 12:35:19 +0000
386@@ -53,7 +53,7 @@
387
388 static int ptr_compare(size_t *compare_length, unsigned char **a, unsigned char **b)
389 {
390- register int length= *compare_length;
391+ register int length= static_cast<int>(*compare_length);
392 register unsigned char *first,*last;
393
394 first= *a; last= *b;
395@@ -68,7 +68,7 @@
396
397 static int ptr_compare_0(size_t *compare_length,unsigned char **a, unsigned char **b)
398 {
399- register int length= *compare_length;
400+ register int length= static_cast<int>(*compare_length);
401 register unsigned char *first,*last;
402
403 first= *a; last= *b;
404@@ -89,7 +89,7 @@
405
406 static int ptr_compare_1(size_t *compare_length,unsigned char **a, unsigned char **b)
407 {
408- register int length= *compare_length-1;
409+ register int length= static_cast<int>(*compare_length-1);
410 register unsigned char *first,*last;
411
412 first= *a+1; last= *b+1;
413@@ -110,7 +110,7 @@
414
415 static int ptr_compare_2(size_t *compare_length,unsigned char **a, unsigned char **b)
416 {
417- register int length= *compare_length-2;
418+ register int length= static_cast<int>(*compare_length-2);
419 register unsigned char *first,*last;
420
421 first= *a +2 ; last= *b +2;
422@@ -132,7 +132,7 @@
423
424 static int ptr_compare_3(size_t *compare_length,unsigned char **a, unsigned char **b)
425 {
426- register int length= *compare_length-3;
427+ register int length= static_cast<int>(*compare_length-3);
428 register unsigned char *first,*last;
429
430 first= *a +3 ; last= *b +3;
431
432=== modified file 'mysys/tree.cc'
433--- mysys/tree.cc 2009-04-17 21:01:47 +0000
434+++ mysys/tree.cc 2009-10-02 12:35:19 +0000
435@@ -103,15 +103,18 @@
436 */
437 tree->offset_to_key=sizeof(TREE_ELEMENT); /* Put key after element */
438 /* Fix allocation size so that we don't lose any memory */
439- default_alloc_size/=(sizeof(TREE_ELEMENT)+size);
440+ default_alloc_size= static_cast<uint32_t>(
441+ default_alloc_size / (sizeof(TREE_ELEMENT)+size));
442 if (!default_alloc_size)
443 default_alloc_size=1;
444- default_alloc_size*=(sizeof(TREE_ELEMENT)+size);
445+ default_alloc_size= static_cast<uint32_t>(
446+ default_alloc_size * (sizeof(TREE_ELEMENT)+size));
447 }
448 else
449 {
450 tree->offset_to_key=0; /* use key through pointer */
451- tree->size_of_element+=sizeof(void*);
452+ tree->size_of_element= static_cast<uint32_t>(
453+ tree->size_of_element + sizeof(void*));
454 }
455 if (!(tree->with_delete=with_delete))
456 {
457@@ -206,7 +209,8 @@
458 }
459 if (element == &tree->null_element)
460 {
461- uint32_t alloc_size=sizeof(TREE_ELEMENT)+key_size+tree->size_of_element;
462+ uint32_t alloc_size= static_cast<uint32_t>(
463+ sizeof(TREE_ELEMENT) + key_size + tree->size_of_element);
464 tree->allocated+=alloc_size;
465
466 if (tree->memory_limit && tree->elements_in_tree
467@@ -308,7 +312,8 @@
468 rb_delete_fixup(tree,parent);
469 if (tree->free)
470 (*tree->free)(ELEMENT_KEY(tree,element), free_free, tree->custom_arg);
471- tree->allocated-= sizeof(TREE_ELEMENT) + tree->size_of_element + key_size;
472+ tree->allocated= static_cast<uint32_t>(
473+ tree->allocated - sizeof(TREE_ELEMENT) + tree->size_of_element + key_size);
474 free((unsigned char*) element);
475 tree->elements_in_tree--;
476 return 0;