unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: master 242a4b49cb: Minor cleanups to X drag-and-drop code
@ 2022-05-20 11:41 Eli Zaretskii
  2022-05-20 12:05 ` Po Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Eli Zaretskii @ 2022-05-20 11:41 UTC (permalink / raw)
  To: Po Lu; +Cc: emacs-devel

> branch: master
> commit 242a4b49cb8397ae4eae0a3d7941e9d5afa16cd7
> Author: Po Lu <luangruo@yahoo.com>
> Commit: Po Lu <luangruo@yahoo.com>
> 
>     Minor cleanups to X drag-and-drop code
>     
>     * src/xterm.c (struct x_client_list_window): Write comments
>     describing the meaning of each field.

Thanks for improving the comments to the code.

> +  /* The event mask for the window before Emacs selected for events on
> +     it.  */
>    long previous_event_mask;

There's something wrong with this comment: I cannot parse that
sentence.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: master 242a4b49cb: Minor cleanups to X drag-and-drop code
  2022-05-20 11:41 master 242a4b49cb: Minor cleanups to X drag-and-drop code Eli Zaretskii
@ 2022-05-20 12:05 ` Po Lu
  0 siblings, 0 replies; 5+ messages in thread
From: Po Lu @ 2022-05-20 12:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> +  /* The event mask for the window before Emacs selected for events on
>> +     it.  */
>>    long previous_event_mask;

> There's something wrong with this comment: I cannot parse that
> sentence.

It's the bitmask of input events that Emacs asked (selected) for
previous to it being changed in `x_dnd_compute_toplevels'.  I see the
problem with the comment now, and will correct it.  Thanks.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: master 242a4b49cb: Minor cleanups to X drag-and-drop code
  2022-05-22  7:50 ` Po Lu
@ 2022-05-22  7:55   ` Po Lu
  0 siblings, 0 replies; 5+ messages in thread
From: Po Lu @ 2022-05-22  7:55 UTC (permalink / raw)
  To: Jashank Jeremy; +Cc: Emacs development list

Po Lu <luangruo@yahoo.com> writes:

> Silly compiler.  If rc is true, tmp_data is always initialized.

Nevermind, should be fixed now.  Thanks.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: master 242a4b49cb: Minor cleanups to X drag-and-drop code
  2022-05-22  6:56 Jashank Jeremy
@ 2022-05-22  7:50 ` Po Lu
  2022-05-22  7:55   ` Po Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Po Lu @ 2022-05-22  7:50 UTC (permalink / raw)
  To: Jashank Jeremy; +Cc: Emacs development list

Jashank Jeremy <jashank@rulingia.com.au> writes:

> Whilst chasing down the build failure against e465ea816d, I observed
> some compiler warnings related to this patch:
>
>     .../emacs/src/src/xterm.c:1994:7: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>       if (rc)
> 	  ^~
>     .../emacs/src/src/xterm.c:2016:7: note: uninitialized use occurs here
>       if (data[1] > XM_DRAG_PROTOCOL_VERSION)
> 	  ^~~~
>     .../emacs/src/src/xterm.c:1994:3: note: remove the 'if' if its condition is always true
>       if (rc)
>       ^~~~~~~
>     .../emacs/src/src/xterm.c:1977:16: note: initialize the variable 'data' to silence this warning
>       uint8_t *data;
> 		   ^
> 		    = NULL
>
> This looks like a plausible issue --- XGetWindowProperty looks to be
> fallible, and in the event it fails we're potentially dereferencing an
> uninitialised value.  I suspect the check introduced in this commit
> should be within the immediately preceding region; however, I do not
> know what the implications of such a change would be, and thus won't
> concretely suggest one.

Silly compiler.  If rc is true, tmp_data is always initialized.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: master 242a4b49cb: Minor cleanups to X drag-and-drop code
@ 2022-05-22  6:56 Jashank Jeremy
  2022-05-22  7:50 ` Po Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Jashank Jeremy @ 2022-05-22  6:56 UTC (permalink / raw)
  To: Po Lu, Emacs development list

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

> diff --git a/src/xterm.c b/src/xterm.c
> index a8745894eb2..a3b7c4ac257 100644
> --- a/src/xterm.c
> +++ b/src/xterm.c
> [...]
> @@ -1977,6 +2012,9 @@ xm_read_drag_receiver_info (struct x_display_info *dpyinfo,
>        rec->byteorder = XM_BYTE_ORDER_CUR_FIRST;
>      }
>
> +  if (data[1] > XM_DRAG_PROTOCOL_VERSION)
> +    rc = 0;
> +
>    if (tmp_data)
>      XFree (tmp_data);

Whilst chasing down the build failure against e465ea816d, I observed
some compiler warnings related to this patch:

    .../emacs/src/src/xterm.c:1994:7: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
      if (rc)
	  ^~
    .../emacs/src/src/xterm.c:2016:7: note: uninitialized use occurs here
      if (data[1] > XM_DRAG_PROTOCOL_VERSION)
	  ^~~~
    .../emacs/src/src/xterm.c:1994:3: note: remove the 'if' if its condition is always true
      if (rc)
      ^~~~~~~
    .../emacs/src/src/xterm.c:1977:16: note: initialize the variable 'data' to silence this warning
      uint8_t *data;
		   ^
		    = NULL

This looks like a plausible issue --- XGetWindowProperty looks to be
fallible, and in the event it fails we're potentially dereferencing an
uninitialised value.  I suspect the check introduced in this commit
should be within the immediately preceding region; however, I do not
know what the implications of such a change would be, and thus won't
concretely suggest one.

    ~jashank

[-- Attachment #2: OpenPGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-05-22  7:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20 11:41 master 242a4b49cb: Minor cleanups to X drag-and-drop code Eli Zaretskii
2022-05-20 12:05 ` Po Lu
2022-05-22  6:56 Jashank Jeremy
2022-05-22  7:50 ` Po Lu
2022-05-22  7:55   ` Po Lu

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).