GNU bug report logs - #76517
31.0.50; feature/igc 6ff509af3d31 crash on Wayland KDE, (with -g3

Previous Next

Full log

Message #41 received at 76517 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: execvy <at> gmail.com, 76517 <at> debbugs.gnu.org
Subject: Re: bug#76517: 31.0.50;
 feature/igc 6ff509af3d31 crash on Wayland KDE, (with -g3
Date: Tue, 25 Feb 2025 17:14:07 +0200

> Date: Mon, 24 Feb 2025 21:01:31 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: execvy <at> gmail.com, 76517 <at> debbugs.gnu.org
> 
> > No.  I'm saying that display_mode_element scans for a '%', finds it,
> > takes the next byte, which is the first byte of the fullwidth comma,,
> > passes it to decode_mode_spec, then leaves offset pointing to the second
> > byte of the multi-byte sequence following the %, and attempts to
> > continue printing the modeline from that offset, in the middle of a
> > multi-byte sequence.
> 
> Maybe this proposed patch will make things clearer.  Please let me know
> whether it's okay for master or how to improve it.  Thanks!

LGTM, thanks.  A few minor comments below.

> A percentage sign followed by a non-ASCII character could previously
> result in a crash.  Ignoring such constructs makes them harmless.

This is better placed in a comment near the code, IMO.

Also, we don't really ignore a non-ASCII character following % any
more than we ignore a following ASCII character X for which %X has no
known meaning on the mode line, right?

So maybe the comment should simply say "% could be followed by a
multibyte character"?

> @@ -27933,6 +27933,12 @@ display_mode_element (struct it *it, int depth, int field_width, int precision,
>  		while ((c = SREF (elt, offset++)) >= '0' && c <= '9')
>  		  field = field * 10 + c - '0';
>  
> +		offset--;
> +		int length = 1;
> +		if (STRING_MULTIBYTE (elt))
> +		  c = string_char_and_length (SDATA (elt) + offset, &length);
> +		offset += length;

Hmm... isn't this better written as

                if (STRING_MULTIBYTE (elt))
		  {
		    int length;
		    offset--;
		    c = string_char_and_length (SDATA (elt) + offset, &length);
		    offset += length;
		  }

Because in the unibyte case we already have the character in C and
offset is already advanced past it, where it should stay.  Or what did
I miss?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.