Skip to content

Topic fix memory leaks#365

Open
aldot wants to merge 20 commits intojordansissel:masterfrom
aldot:topic-fix-memory-leaks
Open

Topic fix memory leaks#365
aldot wants to merge 20 commits intojordansissel:masterfrom
aldot:topic-fix-memory-leaks

Conversation

@aldot
Copy link
Contributor

@aldot aldot commented Oct 22, 2021

plug some memory leaks (ontop of topic-silence-warnings)

aldot added 20 commits October 21, 2021 13:12
italic does not work in verbatim paragraphs, remove it:
 sed -i -e 's/^\(  *.*\)I</\1</g' xdotool.pod

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
.. when allocating space for tokens while parsing.
The array allocation was checked already, also check that allocating
memory for the content works out fine.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
I was getting 'Unknown command: <garbage-on-stack>' for a sequence of
commands that containted no newline:
echo -n "several commands and args here" | xdotool -

fgets null-terminates the buffer, but we were tokenizing past the end of
the read line in the while-loop since we skipped the _final_ zero byte.

There was another bug when tokenizing the commandline arguments when
one token spans more than one buffer, the token was seen as two arguments,
cut in half mid-word where the previous buffer ended and the new buffer
started. E.g.: printf "mousemove%-4083s--polar -- 0 0" " "
complained that "Unknown command: olar".
Another incarnation of the same bug could be observed if you fed a token
into script_main that was longer than the buffer size: It was split on
the buffer boundary, making several tokens out of the single token given
by the user. E.g.:
 perl -le 'print(("X"x 9000))' > input-9000
or
 python3 -c 'print("X" * 9000)' > input-9000
and looking at the script argv when cat input-9000 | xdotool -

Or printf "mousemove%4107s" "--polar -- 0 0 sleep 0.1" > /tmp/i
(cat /tmp/i;echo " sleep 4";echo mousemove --polar -- 30 30) | ./xdotool -

Fix this by correctly handling tokens that cross buffers.

Diagnose allocation failures while at it.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
window_arg was strdup()ed but not freed, fix that.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
SUS specifies that free(NULL) is perfectly valid (a noop).
Remove superfluous guards.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
This is a private helper-function of window_get_arg() and not used
anywhere else.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
and remove now redundant declarations.
A single declaration avoids the risk of diverging scattered
declarations.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
These live in xdo_cmd.h so are redundant in xdotool.c

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
1) warning: no previous prototype for ‘xdo_get_window_classname’
2) warning: pointer targets in assignment from ‘char *’ to ‘unsigned char *’ differ in signedness
   for classhint.res_class

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
72 bytes in 1 blocks are definitely lost in loss record 1 of 1
   at 0x4837B65: calloc (vg_replace_malloc.c:762)
   by 0x4A4D3A0: XkbGetMap (in /lib/libX11.so.6.3.0)
   by 0x110275: xdo_new_with_opened_display (in ./xdotool.static)
   by 0x10D369: xdotool_main (in ./xdotool.static)
   by 0x4D7CBBA: (below main) (libc-start.c:308)

Use XkbFreeKeyboard() to free the data allocated by XkbGetMap().

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
17 bytes in 1 blocks are definitely lost in loss record 1 of 1
   at 0x483577F: malloc (vg_replace_malloc.c:309)
   by 0x49E3EF6: XGetWindowProperty (in /lib/libX11.so.6.3.0)
   by 0x10E7F0: xdo_get_window_property_by_atom (in ./xdotool.static)
   by 0x10F0BD: xdo_find_window_client (in ./xdotool.static)
   by 0x10F17A: xdo_find_window_client (in ./xdotool.static)
   by 0x10F309: xdo_get_mouse_location2 (in ./xdotool.static)
   by 0x111C24: cmd_getmouselocation (in ./xdotool.static)
   by 0x10CCB5: context_execute (in ./xdotool.static)
   by 0x10D3F3: xdotool_main (in ./xdotool.static)
   by 0x4D7CBBA: (below main) (libc-start.c:308)

free the data returned by XGetWindowProperty().

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
xdo_send_keysequence_window_list_do: j is initialized in the for loop,
remove redundant zero initialisation.

_xdo_charcodemap_from_keysym: Remove unneeded local variable len.
Initialize key to zero only if we did not match the keysym.

_xdo_send_keysequence_window_to_keycode_list: Was using the wrong size
to realloc the keys array.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
In function ‘xdo_get_desktop_for_window':
 warning: dereference of NULL ‘data’ [CWE-690] [-Wanalyzer-null-dereference]
 683 |     *desktop = *((long*)data);

Fix this by setting nitems, type and size arguments of
xdo_get_window_property_by_atom to zero.
The callers do not look at a NULL return value from
xdo_get_window_property_by_atom (as they should) but if there are nitems

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Usually we would setup the return value to indicate failure and then
goto out; to use a common cleanup block. But since there is not a single
'goto' in the source ATM i am uncertain if that style would be
accepted here.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
@aldot aldot mentioned this pull request Oct 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant