Re: [Cscope-devel] A dicode compression crash

[Petr S. <pe...@sc...>]

Hi Hans-Bernhard,

Thanks for the fix. However, I would like that you do not use macros and
just stick in the (unsigned char) cast as is. Macros make things more
readable, but much more difficult to debug (if you ever had to debug
ksh, you'll feel the same.)

Thanks again,

Petr
> I've just come across a crash inside putcrossref(), and on debugging it,
> found that it was caused by casting a char to (unsigned) and using the
> result as an index into dicode1[]. That's not quite correct. The cast must
> be to (unsigned char), where as (unsigned) alone implies (unsigned int).
> In the case at hand the signed character was -8, which got casted to 2^32
> - 8. The expected result was, of course 256 - 8.
> 
> So I changed the datatypes and casts in all relevant places from unsigned
> to unsigned char. That fixed the crash nicely.
> 
> While being at it, I also streamlined the dicode compression stuff a bit,
> using macros so it won't affect the performance. Here's their definition
> to go into global.h:
> 
> /* and some macros to help using dicodes: */
> /* Check if a given pair of chars is compressable as a dicode: */
> #define IS_A_DICODE(inchar1, inchar2) \
>   (dicode1[(unsigned char)(inchar1)] && dicode2[(unsigned char)(inchar2)])
> /* Combine the pair into a dicode */
> #define DICODE_COMPRESS(inchar1, inchar2)               \
>   ((0200 - 2) + dicode1[(unsigned char)(inchar1)]       \
>    + dicode2[(unsigned char)(inchar2)]
> 
> And this is what the code using it will now look like (from find.c):
> 
> ***************
> *** 674,681 ****
>                 /* compress the string pattern for matching */
>                 s = cpattern;
>                 for (i = 0; (c = pattern[i]) != '\0'; ++i) {
> !                       if (dicode1[c] && dicode2[(unsigned) pattern[i + 1]]) {
> !                               c = (0200 - 2) + dicode1[c] + dicode2[(unsigned) pattern[i + 1]];
>                                 ++i;
>                         }
>                         *s++ = c;
> --- 675,682 ----
>                 /* compress the string pattern for matching */
>                 s = cpattern;
>                 for (i = 0; (c = pattern[i]) != '\0'; ++i) {
> !                       if (IS_A_DICODE(c, pattern[i + 1])) {
> !                               c = DICODE_COMPRESS(c, pattern[i + 1]);
>                                 ++i;
>                         }
>                         *s++ = c;
> 
> Any objections to checking in such a change?
> 
> --
> Hans-Bernhard Broeker (br...@ph...)
> Even if all the snow were burnt, ashes would remain.
> 
> _______________________________________________
> Cscope-devel mailing list
> Csc...@li...
> http://lists.sourceforge.net/lists/listinfo/cscope-devel

-- 
--------------------------------------------------------
Petr Sorfa                  Senior Software Engineer
Santa Cruz Operation (SCO)
430 Mountain Ave.           http://www.sco.com
Murray Hill 07974
NJ, USA
--------------------------------------------------------
Disclaimer: All my comments are my own and nobody else's
----------------------------------------------------------