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 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 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>
The header 'Xarch' is not part of the standard, it is an internal header
that is already included by the other headers; furthermore it is not needed
directly by the code in wmspec.c.
As it breaks compilation on some platforms (namely Solaris 10), its call is
removed.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Revert patches that moved variables from WMScreen to global level
because this broke X displays with multiple independent screens and
caused dock and clip icons to become mixed up. When managing multiple
screens each screen used to have it's own state/dock and clip. This
commit restores that by reverting mainly the commits listed below (and
those that are invalidated by reverting these) and fixing up later
commits to apply after the revert.
Reverted commits:
f60e65001b Moved 'workspace_name_font' from the Screen to a Workspace object in the global namespace
9e103a46e9 Variable workspace_count moved to the workspace object in the global namespace
e5ae684d02 Variable last_workspace moved to workspace object in global namespace
c610b8d7ce Variable current_workspace moved to workspace object in global namespace
f0c5073600 Array of workspaces moved to the workspace object in the global namespace
9c252988f8 Variable workspace_menu moved to workspace object in global namespace
e86b8dcb2f Clip, Dock and Drawers menu moved to appropriate global namespace
074092f319 Removed WScreen args not used
4a7daf2322 AppIcon list moved out of WScreen
2103fe390b Variable clip_icon moved to clip object in the global namespace
014bc52531 wClipIconPaint appicon argument removed
40e1ea08b8 Varible session_state moved to global namespace
6987d4aa40 Removed WScreen argument
0de3e590ce shortcutWindows moved to w_global
2e64831fb6 Removed unused variable wapp_list
b6423a7b4f wmaker: Moved variable Screen Count into the global namespace
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is fixing the maximize window feature.
As it was reported by Miikka Veijonen, in multiple
screens env wmaker is reserving some space for the dock
even on screen that are not displaying it.
This patch is only adjusting the working area for the head
that is displaying the dock.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
Previously, the whitespace in WindowMaker was inconsistent, with some
indentation using spaces and some using tabs, and a mixture of tabs and spaces
used to align comments at the ends of lines. As a result, patches that touched
this file would often result in warnings from checkpatch.pl.
This patch fixes this so that indentation uses tabs and all other alignment uses
spaces.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch adds the ability to "snap" a window to the top, bottom, or any of the
four corners of the screen. It uses three new helper functions, drawSnapFrame,
getSnapDirection, and doSnap, to reduce code duplication and increase
readability.
It also updates NEWS to indicate the additional directions.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
You can now configure the behavior when dragging a maximized window by setting
DragMaximizedWindow in ~/GNUstep/Defaults/WindowMaker. The options are:
- Move: Move the window and retain its maximized status and geometry (the
current behavior and the default).
- RestoreGeometry: Move the window and unmaximize it, restoring its original
geometry.
- Unmaximize: Move the window and unmaximize it, retaining its maximized
geometry.
- NoMove: Don't move the window.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
Window Maker was not correctly maximizing windows in some cases
"... do not cover dock" enabled with "Dock postion":
Normal - maximizes ok
Auto raise & lower - maximizes ok
Keep on Top - maximizes ok
"... do not cover dock" disabled with "Dock postion":
Normal - maximizes ok
Auto raise & lower - maximizes ok
Keep on Top - maximizes not covering dock
Reported-by: Johann Haarhoff <johann@haarhoff.org.za>
Signed-off-by: Amadeusz Sławiński <amade@asmblr.net>
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch adds the ability to completely unmaximize, i.e., clear the maximized
flag and return to the original geometry, a maximized window that is moved.
This behavior mirrors that of other common desktop enviroments, e.g., GNOME,
Unity, and Windows.
To enable this feature, set "UnmaximizeOnMove = YES" in
~/GNUstep/Defaults/WindowMaker.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
If a user moves a window which is currently maximized, the current behavior is
to keep the window geometry and maximized status unchanged. This can lead to
peculiar behavior. For example, suppose a user maximizes a window to the
right half of the screen (either through the window menu, keyboard shortcut, or
new snapping feature), then moves it, and then attempts maximize it to the
right half of the screen again. Instead of the expected result, the window is
unmaximized and returned to its original geometry.
This patch changes the behavior by clearing the maximization flag when a
maximized window is moved. Then, when a window is maximized and then moved, it
can be maximized again without issues.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch adds the ability to "snap" a window to one side of the screen by
dragging it to that side. It is enabled by setting WindowSnapping = "YES" in
~/GNUstep/Defaults/WindowMaker.
Note that window snapping is automatically disabled if DontLinkWorkspaces =
"NO", as this feature also involves dragging a window to one side of the
screen.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is adding mouse wheel action on clip to switch from
one workspace to another. It's a modified version of the vinelinux.org
(WindowMaker-0.95.6-wheel.diff) which was setting the action on the entire dock.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is updating makeWindowListArray function,
as it crosses the whole window focused list each time
we don't have to bother on checking previous and then next
focused windows, so saving some cycles here.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is adding atomic mouse actions to mouse buttons to:
-focus on previous or next window
-move to previous or next workspace
and adding wheel action to
-switch between windows
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is adding a wWindowFocusPrev() and wWindowFocusNext() functions.
And copying switchmenu.c focusWindow() as wWindowSingleFocus().
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is adding a readMenu function to be called from
readMenuPipe and readMenuFile, saving about 20 lines of code.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is improving the key shortcut labelling in the root menu.
It modifies the GetShortcutString function to save some cycles
as wXModifierFromKey is already doing all the string comparisons.
This patch introduces a new function called wXModifierToShortcutLabel
that is checking the return value from wXModifierFromKey.
Not sure why Control was set as a special key, as keyboards could
have 2 controls but also 2 shift keys.
Signed-off-by: Carlos R. Mafra <crmafra@gmail.com>
This patch is adding the _NET_WORKAREA property
which according to EWMH spec MUST be implemented.
Some application like rdesktop or nautilus are using
it, that's maybe why you encountered some errors like these:
** (nautilus:6457): WARNING **: Can not get _NET_WORKAREA
** (nautilus:6457): WARNING **: Can not determine workarea, guessing at layout
For now, the property is only updated when a workarea is created or destroyed.
This patch is updating the coding style based on checkpatch output
and as suggested by Rodolfo it also inverts the condition test
in getAnimationGeometry fct to return earlier when possible.
This patch is fixing the shortcut label used in window menu.
As now when a 'control' shortcut was used, the label displayed
was 'bug' cause GetShortcutString fct which is calling wXModifierFromKey
fct was waiting for a 'Control' static string not the default one
set to 'Ctrl' string.
This patch is fixing the RenderBadPicture X errors on deiconifying
window using double-click.
Background:
I also experienced this issue these days, after checking it was not
introduce with latest apercu feature I decided to dig into it.
As far as I saw in the archive it was first reported by Rodolfo in June 2012.
Each time a window is deiconify using double-click, the error below
is reported in the session error log:
wmaker(catchXError(startup.c:177)): warning: internal X error:
RenderBadPicture (invalid Picture parameter)
Request code: 152
Request minor code: 7
Resource ID: 0x6000a4
Error serial: 9269
So I decided to track when the issue is happening,
from src/action.c wDeiconifyWindow then in src/icon.c wIconDestroy
then in src/wcore.c wCoreDestroy.
At that stage, all the structures I checked are cleaned up properly
during the icon destroy process.
So I also checked when the issue is generated, I ended in
wrlib/xutil.c RCreateXImage (in the share memory extension code)
XSync(context->dpy, False);
oldErrorHandler = XSetErrorHandler(errorHandler);
XShmAttach(context->dpy, &rximg->info);
XSync(context->dpy, False);
XSetErrorHandler(oldErrorHandler);
At this point I was quite lost, cause it means the error could
have been in the server error queue already.
Facts, seems the issue is appearing only on double-clicking the miniwindow,
but not when "Deminiaturize" is clicked from the right-click contextual menu.
I also checked the code from winmenu.c when the action MC_MINIATURIZE
is selected from execMenuCommand fct, but it was just calling
wDeiconifyWindow function. Hum.
So, after one full day of thought I tested to just change the focus before
double-clicking the miniwindow, and bingo it works.
That means when the icon is destroyed the X server still has a reference
on it that's generating the issue.
