We have a few directories with source codes that we tell configure to
prepare, but we do not actually want them built during normal operations
(tests and examples only).
However, there are some special targets brought by automake which still
need to see them, so this patch adds these directories to the list, but
only for these rules, we keep them unvisited by the normal build process.
The wanted side effect of this is that now "make distcheck" works untill
the end as expectable.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The code is making use of a few of the libm functions, but it looks like
gcc adds automatically the libm dependency (either by trying to be smart or
as an inherited dependency?).
Apparently, when compiling with clang-3.5 the function 'round' still needs
the use of math library (the others do not seem to), so this patch adds it
to the list of link libraries, which is more portable.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
There are a few cases in which nested functions are an helpful way to write
code, as this is explained in 'script/nested-func-to-macro.sh'. However,
some compiler do not support them (like clang), so this patch proposes an
elegant solution, where developers can get the benefit of them, but for
users they are automatically converted to C macro if needed.
The advantage of this solution is that we keep the code simple, there is no
hack in the source (like #ifdef and code duplication), yet still having the
full advantages.
The translation is done according to what have been detected by configure
(see the WM_PROG_CC_NESTEDFUNC macro) so that user has nothing to do.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The memory was allocated with wmalloc, so for consistency it should be
freed using wfree. This could be a problem if the user compiled with
support for Boehm GC, or if we later decide to add support for other malloc
libraries.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, the function handles the case where the file to be
copied is not found by properly warning the user and the deleting the
currently built theme directory, but then it continued executing the file
copy that would crash on the null pointer.
This patch just adds the missing return that will avoid the crash.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Two WMColor were created to draw the background for the title on icons for
iconified windows, in the Appearance panel. As pointed by Coverity, these
colors were not released after use, which this patch fixes.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The name of the color is stored in a newly created PLString which leaks.
This is due to the fact that they are created with a refCount of 1, then
the PLArray in which they are placed increments that count, so at list
destruction the count would return to 1 instead of 0, meaning the
PLString won't be freed.
This patch properly calls WMReleasePropList after addition to the list, so
that the count goes back to 1, which means it will be automatically freed
when the PLArray will be released.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, if dock is still null at this point then the
function will return via the previous check, because it is not possible to
have icon_pos >= 0 if dock is null.
This patch removes the check because it complicate the code which is not
recommended for maintainability.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Using a variable to store the return value that will be used later is not a
good idea, because it forces to track everywhere in the function when
needing to work on the function.
This patch removes the variables and places explicit return if each case,
so on first look it is clear where the code stops.
It also fixes a bug where the function would handle an event but still
returns False (meaning the event was not treated), whose root cause was
coming from the complexity brought by the variable.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Coverity complain that there can be security issues because the variable
'i' is being modified using untrusted data (coming from a file). This is
probably pessimistic, because in the present case we're talking with the
kernel.
Using the correct signedness for the variable should however keep us safe,
and (I hope) make Coverity happy.
Took opportunity to include an error message in case of read problem
because it can help to debug.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, we store a value into the variable 'entry' but we
never use it later, although we re-use the variable.
For the maintainability of the code, it is not really good as it could
mislead into thinking the value could be used, so this patch removes the
assignation.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Coverity warned because the 2 strcpy may overflow the target buffer (the
code relies on a constant for the max allowed workspace name length).
As in both cases the temporary copy is not useful because the temp copy
will be strdup'd just after, this patch removes the temporary buffer and
uses directly the string.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
In case of problem with a directory when building the list of files for the
Icon Chooser dialog, an error message was generated using a temporary
allocated buffer. This is not really good for memory fragmentation, so this
patch re-uses the local buffer which will be enough for all reasonable
cases.
Took opportunity to make message less prone to translation difficulties,
and include more information about the problem to the user so he may know
what went wrong.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, there is a possible (yet improbable) buffer overrun
when building the list of files to be used in the Icon Chooser dialog.
Better safe than sorry, let's use the safer function to build the complete
name, and add a little message to the user in case of problem so at least
he can know something was not right.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, there were a number of copies done into fixed-size
buffer, it's safer to use the function that sets a limit on the size to
avoid a crash.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, there is in the same procedure a check for null
pointer before use, and later a direct use without check.
In most case, the pointer is not null, but it one case it is hard to be
sure that the pointer cannot be null, so it's safer to just add a check
there also.
Took opportunity to swap the order of the arguments in the if, because if
the config flag is false it is not necessary to spend time on doing the
more expensive strcmp.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, there might be some problems due to sign extension
when performing the shifts and ors operations when converting the RImage to
the format expected for the WM_ICON property.
This patch try to improve things by using as much as possible unsigned
types and by using explicit types conversion instead of counting on the
wrong implicit type conversion done by the language.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, the array created to temporary store the list of
Atoms used in the function 'requestHandler' was leaked.
Because this array is very short lived, there is no need to allocate memory
for this, it just participates in memory fragmentation. Instead, we use now
memory on the stack which is more efficient.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The "documentation" of the function claims to check for the resource in a
number of path, but factually if the application did provide its argument
list when creating the WINGs App structure (which is likely) then the
search would stop before checking all paths.
The code now continues as expectable if the resource was not found in the
path. Took opportunity to avoid a temporary allocation that participated in
memory fragmentation.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As reported by Charles Philip Chan, WPrefs would get into infinite loop
when the support for Pango is enabled.
The problem is due to long strings that are broken into multiple lines by
WINGs. This is done in an iterative process in the internal function
'fitText'.
In order to avoid the cost of duplicating many times the sub-strings, the
functions involved do not place a NUL at the string-splitting position, but
they rely instead on giving the length of the string as a parameter.
The code that checks the Pango text (to avoid re-submitting the string when
not needed) did not use that length, so it would always keep the original
string that is too long, so the fitText function would always receive the
same result and loop forever trying to find where to split the string.
This patch adds the check on the length, so Pango is given the appropriate
string for its pixel size calculation.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Germán Arias, some windows in GNUstep did not have a title
bar because they declare themselves with the NET_WM type MENU.
A closer look at the spec shows that this type is meant for stuff like tear
off menus, and thus like for toolbar it is not expected that the
application does not want a titlebar, as opposite to DROPDOWN_MENU,
POPUP_MENU and COMBO.
So, this patch moves the case for TYPE_MENU to be the same as TYPE_TOOLBAR
and remove the flag that disables having a title bar.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, the function blindly trust the data read from the
file, but in case of problem (corrupted file, not enough memory) it could
behave badly.
This patch adds a check for the depth, counts on RCreateImage to check the
width and height, and in any case it now includes a message for the user in
case he would like to understand what's wrong.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, if the Keysym name is very long and many modifiers
are used, the function 'capture_shortcut' could overflow its internal
buffer.
As the case is very unlikely to appear, do not increase the size of the internal
buffer (it seems to be already well sized, and we have no know maximum size
for a Keysym name), just use the appropriate function to append the name
at the end.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
Unfortunately this will lower the translation score for German, but the
translation guessed by fuzzy matching in gettext is not acceptable in the
current case: the translated string is already used for another entry in
the list, meaning that user would not see the difference between the two
and would probably then not understand why things do not behave the way he
expects.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the sizeof used was not done on the right type.
This worked because the element was a pointer all pointers types have the
same size in most platforms.
For code maintainability, the code will now take the size from the first
element of the array to be sorted, so that if the structure gets changed
someday the expression will stay valid.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
There is a check a few line above that already return from the function
because there is nothing to do for that case, so the case statement cannot
be reached. As not all case are covered in that switch anyway that won't
make a difference, and because an 'int' is used instead of an 'enum' the
compiler will not be able complain anyway.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
The warning message was duplicated many times for each possible case, but
it could have used the same message everywhere, which ease the work of
translators. Took opportunity to include the complete command being
processed so that the user will know more about the problem and may be able
to fix it.
Updated the French translation to show the gain, but not the other
languages because it require more knowledge than what Google Translate can
provide me.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As coverity pointed, the duplicated string was never freed. Considering
what is done with it, is not necessary to allocate a duplicate for it, it
is a waste of time and participates in memory fragmentation.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As a previous patch modified some instances of the error message to include
more information to the user, it is a good idea to update also the other
uses of the message, so that:
- people helping on translation will have less messages to translate
- this mean we provide more information to the user in these places too,
which can help him solve the problem
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As Coverity pointed, when building the command line to execute the current
code just assumed that it would fit in the large buffer. If user were to
provide a line too long, this would crash.
Factually this is probably not possible at current time because the command
given to the function was actually already limited to the MAXLINE size when
it was read, but this may not be guaranteed in future evolution.
Better safe than sorry, so the patch implement a size check when appending
strings, using a more efficient method (strcat re-parse the destination
string from the beginning to find its end every time).
Took the opportunity to:
- not include a trailing space at the end of the command
- do not run command if it was truncated (it could be a problem) but
provide a message to the user about it, so he may fix the problem
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As coverity found a number of problem in the code, a few changes are made
to the function:
- allocate better sizes for the strings (the original code allocated too
much room in many cases and missed the room for the final '\0' in a case)
- do not free strings if empty anymore (the actual check was not correct
anyway), but avoid allocating in first place if it is not necessary.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
Coverity pointed that in the typical code path the function FindImage would
be called twice, leading in leakage of the allocated result from the first
call.
This patch updates the condition so that the case won't arise.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the pointer cannot be null, otherwise the code
would have crashed earlier. As the code seems to always set a valid
pointer, there's no need to make a check there.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
When creating the environment variable for the sub-process that wmaker can
create, Coverity pointed that if was possible to crash if the name of the
display did not contain the ':', which is probably ok in most case, but we
can't be sure about what it could contain in special cases.
This patch adds a proper check so, at least, it would not crash if the case
were to arise.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, despite the numerous null pointer checks there is
still a case where one can pass trough and make wmaker crash.
This patch simplifies it all but making only one check at the beginning so
the code is safe and the remaining is simpler.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, text cannot be null in this part of code. The
analysis shows that if it were, the function wIconChangeImageFile would
have directed execution to the 'else' branch. (and if it hadn't, the code
would have crashed beforehand because 'strlen' does not like null pointers)
Coverity recommends to remove the unnecessary check for code the
maintainability.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
The variable is assigned a value that never change, so it adds extra
complexity which is not good for code maintainability. It is probable that
this was meant for cases that are handled in current code with early
function return, which are better for code readability.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the 'cmd' is checked for null at the beginning of
the function, so the second case to handle the null pointer is not needed.
This also means that 'command' cannot be null (wstrdup never returns null)
so the code can also be simplified.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
Try to provide better messages to understand what went wrong, including
more information, and made them translatable;
Changed the call to 'dup' into 'dup2' which has a safer behaviour because
we can specify the target descriptor we want;
Removed a few check for the 'close()' because in these cases we do not
really care if they fail (we can't do anything about it and it just adds
noise in the logs).
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the file descriptor used in 'dup' to become the
child process's STDIN is leaked, because it will not be used anymore, so we
close it after the dup.
Similarly, the file descriptors that represent the other ends of the pipe
for each process are useless, so let's close them too to keep a reasonable
number of opened file descriptors over time.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
In order to make code easier to maintain, the code related to creating the
Helper process (which helps by setting the background of the workspace) is
moved to a dedicated function, which have been moved to the same location
as the function for communicating with the helper.
Took opportunity to de-CamelCase the related names.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
There was a probable bug when reading settings, because the function used
was 'getInt' which would try to store the result in a 'char'. As it would
be probably easier for user to have the value directly in pixels, the
storage is now done in an int so there won't be problem anymore.
Changed the behaviour of the constant APERCU_BORDER, which would be assumed
to be the size of the border in pixel, but in previous code it was actually
the sum of the two border (1 on each side). All maths have been changed to
have it as a single border width.
Took opportunity to group variable assignation for titleHeight and
shortenTitle in a single place, because it is not convenient to have them
spread around (one value in the beginning and others later in the code) and
using default values prevents some checks that modern compiler can do to
help produce safer code.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the return value is saved into a variable, but this
value is never used. As the variable is re-used afterwards, this
assignation could mislead on what is done, so for code maintainability the
value is just ignored.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, there was a check for null pointer on
scr->dock_dots, but this check was not made in the 2nd use of it, done if
the HIDDENDOT feature was enabled.
Investigation show that it is not possible that this pointer could be
created NULL, so let's remove the unneeded check.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, it is possible that RCreateContext fails for more
reasons that were handled by wScreenInit, so we provide an error message
for the other cases along with cleaner return from function.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the function GetShortcutKey was allocating memory
for temporary operation, and did not free it in the end.
Because it participates in memory fragmentation and it is not really
efficient, this patch removes the allocation and uses a local storage on
the stack, and replaces wstrappend in favour of a fast string build.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
A type conversion applied to a function when used as a pointer is dangerous
because the compiler may not be able to make sure arguments will be
compatible across architectures, so it can crash the application.
This is replaced by a function prototype matching exactly what is expected
for the callback, and have the type conversion on the argument done inside
the function so the compiler have complete liberty to generate any code
needed to handle it safely.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, the function makes use of a pointer which may be
null, so we have to properly check that to ensure application will not
crash.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
