For code maintainability, it is better to have a single definition of the
constant than many ones dispatched in many places. In addition, we try to
count on the constant M_PI that can be defined by the header <math.h> if
possible because it may have the best accuracy for the platform.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, the 4 constant the define the corner positions are not
used anywhere in the code, and it looks like it has always been like this.
As it is a cost for maintainability, get rid of them.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, a few constants which contain sizes for buffers are not
used anywhere in the code. This is probably due to code cleanup, where
these buffers have been removed or where size is automatically calculated
by using 'sizeof' instead to reduce risk of bugs.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
There is a header that defines the stucts and constants to support GNUstep
specific things, and it is already used somewhere else in WINGs, so instead
of duplicating locally the stuff we re-use the header, it makes code easier
to maintain.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The group of XDND_* macro do not bring a lot in term of clarity, so it is
probable that they will get away someday, but at current time as 2 of them
are not used anywhere they get removed to prevent their use in the future.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
This macro was define a *very* long time ago, in commit
d98f1fa645
but was not used at that time, and have never been used anywhere since
then.
As the macro does not look like a good idea for performance anyway, get rid
of it before anyone could get tempted to use it.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
It seems the code had the possibility to load a default icon, probably to
be used on the windows if the main program does not provide any.
This code however have never been enabled, probably because it is better to
not provide an icon and let the window manager use its own.
This patch then removes that dead code, but keeps the image as part of the
WINGs resource for the case where an application would have been using it.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, this macro is never used in the code; when no font is
defined by user the 'paintButton' code already falls back to the
'normalFont' of the screen, and this code could not need the macro anyway.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, this constant is not used anywhere, and it looks like
it have always been the case. As there's no reason to specifically add a
border to this widget, remove the constant.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, the height is never used and the code make use of the
SCROLLER_WIDTH constant anyway, so this patch updates the code to directly
use that constant instead of intermediate values that just adds noise for
code maintainability.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by gcc, the macro is not used, that is probably because it is
totally ok to never provide a title from X point of view, so there is now
reason to fall back to that empty constant.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Because the use of trampolines can be a lot of problems, this patch changes
the use of the flag to:
- make it an error if possible, so coders are forced to not use them;
- always enable them, not just for debug, so we increase the probability
to get them caught.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Strict prototype are better for portability and to avoid bugs because it
makes sure the compiler has the information to properly validate the
arguments given when a function is called.
This flag however need special care when checking for it, because the
declaration for 'main' generated by autoconf cannot be a strict prototype
so the detection would always see the flag as failing.
This patch handles this by creating a dedicated macro for this detection
which uses a good prototype because we're in a case where it is possible,
so the detection will not always fail; it also makes sure to add the flag
to CFLAG only at the end, to avoid falsely crashing any further test done
in the configure script.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
This flag is always detected as unsupported because every time autoconf is
generating a test program for any feature, it puts at the beginning of the
test source all the '#define' that have been detected so far, which is what
we expect normally.
But for this option, as we cannot reasonably make a dummy use of every
macro, the warning triggers and falsely gets autoconf into thinking it does
not work.
This patch creates a dedicated macro (WM_CFLAGS_GCC_OPTION_UNUSEDMACROS)
for this flag, which works around the problem by having no '#define' in the
test source. It also adds a new macro WM_CFLAGS_GCC_OPTION_POSTPONED
because if the flag works, we still cannot add it to the compilation
command because it could fail all further tests done, so the macro will add
it to CFLAGS only at the end.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Apparently, some pixels from the close button on the window image were
missing from the Non-opaque image, they have been added in the XPM file
which was then converted to TIFF with ImageMagick's command:
convert -depth 8 -compress lzw xpm/nonopaque.xpm tiff/nonopaque.tiff
to have a file similar to what was there before
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As pointed by Coverity, the PLStrings created to store the information on
the texture leak. 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 PLStrings won't be freed.
This patch release the PLStrings once after adding them to the PLArray so
the count will go back to 1, which means they will be properly freed when
the PLArray will be released.
Took opportunity to remove the call to WMRetainPropList on the titem-prop
because it artificially increases the refCount but this is already done
when adding to the PLArray.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Recent version of perl seem to report incorrect stuff in regular expression
and this seems to be fixed in the kernel reference file, so this patch
brings the fix to our (older) version so we won't get that spurious message
when running the script.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
A macro can be a source of problems, because the compiler has no type on
the arguments to make checks. Using an inline function allows to do those
checks, meaning clearer error messages, it provides clear info in case of
name collision, it is easier to maintain (no need for the hacky '\' for
multi-lines) and the scope of visibility can be controlled more easily (no
need for #undef).
The macro store_modifier had to face a slight change because its 2nd
parameter is used as a reference, which is now clearly visible in the
prototype of the function.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
A macro can be a source of problems, because the compiler has no type on
the arguments to make checks. Using an inline function allows to do those
checks, meaning clearer error messages, it provides clear info in case of
name collision, it is easier to maintain (no need for the hacky '\' for
multi-lines) and the scope of visibility can be controlled more easily (no
need for #undef).
Took opportunity to change a 0 to the constant NoSymbol which is the name
defined by X for this case and another to NULL which is the right way to
set a null pointer in C.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
A few files are generated using custom commands, which so far did not
follow the silent rule as the compilation stuff.
This patch adds the needed stuff so they will also be silent if the user
wants so, leading to a cleaner build process where warning/error messages
are more visible.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The function is called only if wsmap is not null, and the function is not
modifying its value so it won't become null. Removed the checks to keep the
code as simple as possible for maintainability.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The definition in the local header was not correct; it works because gcc is
tolerant to this kind of errors but other compilers are not. The
declaration was creating a local variable in each file that call header,
and because it is not static gcc's linker will merge them. Other compilers
will at best complain for duplicate symbol, and at worst silently duplicate
the variable so it will not work as expected.
The variable is now moved to the existing structure meant for global
variables, so now the code is really clear about using a global variable
instead of a static/local one.
Took opportunity to add some missing 'static' attributes to some variables.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
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>
The function is building strings from the directory names into an allocated
buffer, but the function took time first to calculate the exact size needed
for the resulting string, so the check on wstrlcat's result will never
fail.
As we still use wstrlcat it is not possible to overrun the buffer, we would
just return a truncated string in the list instead of return no list at all
but the case where it would happen is impossible.
This should fix Coverity #50111 (Resource leak) which was present in the
code of one of the related early return.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
The original choice may have looked mathematically correct, but it was
actually counter-intuitive and opposite to what every other application
do with sliders.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
When creating the temporary file that will become the final file if no
problem occurs, there is a chmod done which does not give write access to
the group and to the others, but this is the task of the user-set umask.
This patch makes the rights to everything (except execution, of course) and
still applies the umask, so in the end the file will have the rights that
user wants.
Took the opportunity to make a little change related to the umask: it seems
that some version of mkstemp have a security issue, which is in not a
problem in our use case, but Coverity reports it (#50201) so as it does not
cost anything, the patch also fixes it with an appropriate comment to
explain the situation.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
The original code used the libc "fopen" kind of operation, which are handy
when manipulating text files, but:
- bring an overhead for binary files that we don't need here;
- does not provide the mechanisms for safe error handling and special cases
As Coverity reported a Time-of-Check/Time-of-Use type of security issue,
took the opportunity to fix it and increased the size of the buffer used
for data to allow better use of modern disk performances.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
As pointed by Coverity, there were some null pointer checks that had been
misplaced, due to a pointer dereference present in a preceding check. This
had been fixed by adding another null check in the check, making a
duplicate check.
This patch moves the null pointer check in first place, and remove the
pointer check from the range check to separate the pointer check on one
side and the range check on the other side.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
It is more efficient to use the dedicated function than to call an external
binary program to do the job, and it reduce the risk of problem in case the
path would end up with potentially problematic characters.
It should also close Coverity bug #50225 ("Use of untrusted string value")
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The International System of Units defines "ms" as the standard abbreviation
for milliseconds, so let's use it everywhere to be consistent.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
It is general practice to not consider the strings displayed in a dialog as
real sentences, so this patch removes the dots at the end (including in the
translation);
Took the opportunity to fix some translated string that were probably not
found anymore to due other changes;
Moved a few widgets to make everything look better centered;
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The goal was to reduce the risk of truncated lines in the list of actions
(a few of them are a bit long - and need to be!), so it was an opportunity
to align the widgets with consistent spacing.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
It is general practice to not consider the strings displayed in a dialog as
real sentences, so this patch removes the dots at the end (including in the
translation);
Took the opportunity to make a light cleanup of the translations in the
concerned areas, and fix some translated string that were probably not
found anymore to due other changes;
Make small reorganisation of the widgets to reduce the risk of truncated
strings, with keeping the overall clean aspect of the panel;
Took the opportunity to clarify the label related to the colormap to avoid
misunderstanding, and include a balloon text to clarify its reason-to-be
because, as modern display tend to all be TrueColor, it may not be
intuitive to new user what it's for.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The goal is to reduce the risk for truncated label, and take the
opportunity to align everything with consistent spacings.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Due to a long history of evolution, it was time to rethink the placement
of all widgets to achieve a clean look. The frames have been reshuffled to
be aligned to each other, and as a bonus the Mod+Wheel setting now has its
own frame instead of being part of another unrelated frame.
Took opportunity to include an info balloon to explain what the "by
keyboard" checkbox actually means.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As many option have been added to it, the panel started to look overloaded.
With a little rework, it now displays the same options in a cleaner way.
Took opportunity to include some balloon help for the AppIcon option to try
to be clearer than what the short label allows.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As many option have been added to it, the panel started to look overloaded.
Content have been slightly moved to make it look nicely organised inside
the window, and with possibly less truncated text.
Took opportunity to include a visual feedback to the user, when checking
the "Disable mouse actions" box then we disable the related popup menu
buttons to reflect this.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As the data to create the icons for the dock configuration was already
grouped in a structure, it is a good idea to also include the balloon text
which is linked to them in the array, so the code is simpler and safer.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As the code is already treating these frames as an array, it is a good idea
to do the same for their titles so it is possible to reuse its size at
convenient places to make the code safer.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
In the panel for Appearance configuration, there are some sample colours
proposed to users; the code used to have a bunch of constants hard-coded
to handle these colours.
This patch replace all constants with the macro wlengthof to use the number
of element in the array determined by the compiler, so modifying the list
won't cause hidden bugs. This include a somehow smarter code to dispatch
the colours in the window to automatically arrange them cleanly whatever
their number is.
Took opportunity to de-CamelCase the name of the array as it did not add
complexity in the patch.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Took the opportunity to de-CamelCase the name of the variable to comply
with project's coding rules.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync. It also makes the code simpler.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Having all these information spread in different places makes it error
prone when wanting to add/remove/change something in the list are there are
many unrelated places to keep in sync.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
This reduce the risk to miss something in case the array they refer to gets
updated, because with the macro the proper number of element will be
evaluated by the compiler automatically.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Because the macro wlengthof preforms a check of validity of its argument
with static_assert, there is a use case where it fails with a compiler
error. This patch introduces an alternate macro without the check, to be
used only for this specific case.
To be able to use the size of the array, a few of those array declaration
have to be moved before the structure in which their size is used.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As reported by Andrew, the compilation of the WRaster broke because
there was an API change in libgif v5.1 versus the v5.0 (something had been
forgotten for DGifCloseFile to be easily used in wrappers for dynamic
languages).
Now, if we have detected that we're in 5.x release, we use the GIFLIB_MINOR
macro to see what the function prototype is (this macro was introduced only
in 4.1.6 so we cannot fully rely on it to detect the version of the
library).
The possible error code is not used because at the place we use the
function we would not be able do do anything more meaningful with it.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As reported by Milan Čermák, using this variable breaks compilation on
Solaris, because it is a hack which is not standard. To ensure portability,
we now rely on main's argv[0] which is always available.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Christophe CURIS