Suspicious code in search.c

Suspicious code in search.c

[Kim F. Storm]

See search.c line 1711-1712:

	  if (ASCII_BYTE_P (*ptr) || ! multibyte)
	    ch = *ptr;
-->	  else if (charset_base
-->		   && (pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1]))
	    {
	      unsigned char *charstart = ptr - 1;


Based on the indentation, I would assume the following interpretation
is intended:

	  else if (charset_base
		   && ((pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1])))
		      ^                                            ^

Can somebody who knows this code better than I take a look (and
perhaps reformat the code if the current code is ok).


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Suspicious code in search.c

[Richard M. Stallman]

    See search.c line 1711-1712:

	      if (ASCII_BYTE_P (*ptr) || ! multibyte)
		ch = *ptr;
    -->	  else if (charset_base
    -->		   && (pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1]))
		{
		  unsigned char *charstart = ptr - 1;

Handa, you wrote that code in April.  Can you clear it up?

Re: Suspicious code in search.c

[Kenichi Handa]

In article <E1ESjzL-0004ZL-8D@fencepost.gnu.org>, "Richard M. Stallman" <rms@gnu.org> writes:

>     See search.c line 1711-1712:
> 	      if (ASCII_BYTE_P (*ptr) || ! multibyte)
> 		ch = *ptr;
--> 	  else if (charset_base
--> 		   && (pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1]))
> 		{
> 		  unsigned char *charstart = ptr - 1;

> Handa, you wrote that code in April.  Can you clear it up?

I've just done it.

---
Kenichi Handa
handa@m17n.org

Re: Suspicious code in search.c

[Richard M. Stallman]

    > Handa, you wrote that code in April.  Can you clear it up?

    I've just done it.

Thanks.

Re: Suspicious code in search.c

[Kenichi Handa]

In article <m37jc8i6l1.fsf@kfs-l.imdomain.dk>, storm@cua.dk (Kim F. Storm) writes:

> See search.c line 1711-1712:

> 	  if (ASCII_BYTE_P (*ptr) || ! multibyte)
> 	    ch = *ptr;
--> 	  else if (charset_base
--> 		   && (pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1]))
> 	    {
> 	      unsigned char *charstart = ptr - 1;


> Based on the indentation, I would assume the following interpretation
> is intended:

> 	  else if (charset_base
> 		   && ((pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1])))
> 		      ^                                            ^

Yes, and a compiler should interpret that part as above.
But, I agree that it's better to add that explicit parentheses.

> Can somebody who knows this code better than I take a look (and
> perhaps reformat the code if the current code is ok).

Done.

---
Kenichi Handa
handa@m17n.org

Re: Suspicious code in search.c

[Nick Roberts]

Kenichi Handa writes:
 > In article <m37jc8i6l1.fsf@kfs-l.imdomain.dk>, storm@cua.dk (Kim F. Storm) writes:
 > 
 > > See search.c line 1711-1712:
 > 
 > > 	  if (ASCII_BYTE_P (*ptr) || ! multibyte)
 > > 	    ch = *ptr;
 > --> 	  else if (charset_base
 > --> 		   && (pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1]))
 > > 	    {
 > > 	      unsigned char *charstart = ptr - 1;
 > 
 > 
 > > Based on the indentation, I would assume the following interpretation
 > > is intended:
 > 
 > > 	  else if (charset_base
 > > 		   && ((pat_end - ptr) == 1 || CHAR_HEAD_P (ptr[1])))
 > > 		      ^                                            ^
 > 
 > Yes, and a compiler should interpret that part as above.
 > But, I agree that it's better to add that explicit parentheses.

The original expression is equivalent to:

 	  else if ((charset_base
 		   && (pat_end - ptr) == 1) || CHAR_HEAD_P (ptr[1]))

I don't know which expression is right, but you've changed the logic.

Nick


main ()
{
  if (0 && 0 || 1) printf ("1\n");
  if ((0 && 0) || 1) printf ("2\n");
  if (0 && (0 || 1)) printf ("3\n");
}

nickrob/38 mytest
1
2

Re: Suspicious code in search.c

[Kenichi Handa]

In article <17240.19436.738186.836546@kahikatea.snap.net.nz>, Nick Roberts <nickrob@snap.net.nz> writes:
> The original expression is equivalent to:

>  	  else if ((charset_base
>  		   && (pat_end - ptr) == 1) || CHAR_HEAD_P (ptr[1]))

> I don't know which expression is right, but you've changed the logic.

Oops, you are right.  I misunderstood.  Yes, I changed the
logic and I think the change is correct.  This is the place
to check if *ptr is the last byte of a multibyte character.
It is so if *ptr is the last byte of the whole text or
ptr[1] is the first byte of a character.

---
Kenichi Handa
handa@m17n.org