Topic silence warnings#364
Conversation
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>
jordansissel
left a comment
jordansissel
left a comment
There was a problem hiding this comment.
General comment: There several commits that make what seem to be unrelated changes. Combine this with the PR subject "Topic silence warnings" with almost no information in the PR description, and it feels like not enough detail.
In the future, I'd love to see more details about the intent of the change (silence what warnings? When do the warnings show up? How do these changes improve things? etc).
That said, I do like the code cleanup, thank you!
I will also test the script changes before merging.
|
I wrote a test case for the script buffer changes 👍 |
Where "long" means longer than the script buffer size which, at this time, is 4096 bytes. Verified this test case fails prior to #364 being merged and succeeds after.
|
Added a test case: a3f4994 |
These were meant to be 3 distinct patchsets. If you checkout https://github.com/aldot/xdotool.git and look at the history via
The 4 "silence warnings" patches are: and should each contain a rational for the change.
You're welcome. |
Another good test would be to span an argument to the next buffer: is the expected behaviour. I think that would be a good testcase; If you don't like sleep the any other command will of course do to demonstrate the issue. |
* xdotool.1: Fix rendering of example commands
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>
* Error out on allocation failure
.. 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>
* Fix tokenizing a single-line command chain
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>
* fix memory leak in mousemove, mousedown
window_arg was strdup()ed but not freed, fix that.
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
* Remove superfluous guards around free(NULL)
SUS specifies that free(NULL) is perfectly valid (a noop).
Remove superfluous guards.
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
* window_is_valid(): make static
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>
* context_execute(): Move prototype to xdo_cmd.h
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>
* xdotool: Remove redundant declarations
These live in xdo_cmd.h so are redundant in xdotool.c
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
* getwindowclassname: silence warning
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>
Where "long" means longer than the script buffer size which, at this time, is 4096 bytes. Verified this test case fails prior to jordansissel#364 being merged and succeeds after.
2 participants
Silence some warnings.
Thanks!