According to the way its value is being used everywhere, that is
what would be expected, so let's make it official.
Please note that this may introduce warnings on user code using
this function ("...discard const...") but that's an opportunity
for them to check that their code is not doing anything wrong.
The internal function 'unescapestr' is used to transform strings which
may contain escape sequences (\x) into their plain representation.
There are a few cases where the function can misbehave (typically parse
after the end of string, thus writing past the end of the reserved
result area) which can be a source of problem later. The new code
should be safer.
The previous syntax used an explicit cast to remove the CONST
attribute, but the right way is to keep the attribute and store
the result in a variable which is defined properly.
This makes the WUtil API as much const-correct as possible for
the arguments being given to its functions.
This does not make it totally correct as it does not changes the
const-ness on returned values because the goal of this patch is
to make no visible change to existing program that would use this
library.
Note that the argument is also stored as-is in the PLData structure
but only for debugging purpose (warning display to user), hence the
choice to not duplicate it. As a side effect, it was 'const'-ified
too to reflect that.
As a side note, in 'wfindfileinlist' the first argument should be:
const char * const *path_list
However, due to limited support for const in plain C, that would
introduce warnings in user code. For compatibility issues, this
was not implemented.
A number of functions do not actually modify the strings given as
parameter, but only read or duplicate it. In this case it is a good
practice to mark that parameter as pointer-to-const to let the
compiler known about it, to be able to perform appropriate
optimisations.
Prototype of function changed in commit
d1e1c13fa3
but two of them were not updated in the source.
Thanks to Rodolfo García Peñas for taking the time to test and report.
The addition of 'const' to parameters of library's functions is an
API change, although this will break neither the binary interface
nor the compilation of program using the library; the other changes
also have no impact on the compiled library object.
All these functions expects agruments like color or list-of-points
that should not be modified (and are not) by the function; added
the corresponding qualifier to reflect that.
An enum is always a better idea as it allows the compiler to do
some checks, and as this info is internal only to the WRLib it
will not change the API.
The functions are declared in different files but they were called
in another file which re-declared the prototypes. This is dangerous
as it can lead to misaligned prototypes when functions changes.
They are now grouped in the library internal header 'imgformat.h'
The config-file-keyword was defined in an array, but the user displayed
string was defined at another place, thus requiring special care when
modifying the list, to keep the same number of entries, in the same
order, ...
This was an open call for subtle bugs, so the keyword and user string
are now at a single place, making updates on the list easier and the
GUI generation code smaller.
As proposed by Christian Schulte, when compiling WindowMaker on BSD
it uses the kvm library to gather information on processes (in the
os-dependant file), so we must properly link against the corresponding
library, which this patch adds.
The list of options in the Expert tab of WPrefs.app was filled using
hard-coded list of commands with static indexes and the related
usage were spread accross procedures. This was dangerous because
it could lead to mismatched indexes, the list length being also
variable, so it was hard to update.
This patch proposes to group all the options in a single place (a
static const array), so it is easy to add/remove/reorder the options,
and the code just parses this array based on its auto-calculated
size.
As reported by Amadeusz Sławiński, there were a number of debug-only
information printed by the menu parser, which had no reason to be
in a user package.
This patch removes all the hard-coded DEBUG definitions pointed
by Amadeusz and adds a new (standard-like) option '--enable-debug'
to the configure script, which activates debug stuff for the devs.
The default behaviour is now to not have them.
As a side effect, the option also disable optimisation, which are
generally annoying when trying to run a debugger.
As found by Rodolfo, it looks like there could be memory leak in the
function 'RSmoothScaleImage' because it reserveda given number of
memory blocs but used another count to free them after use.
This patch uses the same count for release as it seems this variable
is not modified in between. Took the opportunity as Rodolfo proposed
to convert a global variable to a local variable - this global
definition seems incorrect.
As reported by Amadeusz Sławiński, the support for (d)quoted string
in the menu file was not exactly the same as in previous versions
of WindowMaker, so we had a regression which is not acceptable.
This patch propose a consistent handling for (d)quoted text instead
of the previous on-the-title-only code; now all (d)quoted strings
are still kept as-is (as expected) but the (d)quotes are always
removed.
As a side note, it also improve the support for \escaped character
to work correctly as mentioned in the example of the default menu.
Note: This reverts b2e7620868 which
was an incomplete solution to the problem.
As reported by Rodolfo, there are some cases when working with icons
where a crash can occur, which is related to trying to re-release
some memory that have been already freed previously.
This patch adds a reset-to-NULL of the corresponding pointers so that
on next usage wmaker will know there's no more memory associated with
these pointers.
The code was setting some values in the window's attribute structure
which were not being used (missing the corresponding vmask flag) and
was setting some vmask bits without setting the corresponding
value in the structure.
The new function 'unpaint_app_icon' checks its argument 'wapp' aginst NULL,
however it does dereference the pointer beforehand, which will end up in
a crash if the null-pointer case occurs.
As found by Rodolfo, a crash could happen when changing an icon to
a non-existent file. From his report:
Steps To Reproduce
1. Open the Settings box, for example for WMDock. (Right click on WMDock, Settings).
2. In the "Icon Image" field, write some characters. For example "asdfasdfasd".
3. Click in "OK". You get "Could not open specified icon file: GNUstep.tiffasdfasdfasd"
4. Hit in "OK"
5. Repeat 2,3,4 some times (sometimes 1, sometimes 2, sometimes 10).
The problem came from freeing the previous icon but not updating
the pointer, so if the new file did not exist it ended up with a
fake-looking icon being used, hence possible crash.
BTS: http://www.kix.es/mantis/view.php?id=1
When generating the full path+name of file to search for a file
being #included, it was generated in a buffer that's supposedly
large enough (MAXLINE > 2*PATH_MAX). However, this limit has a few
issues (PATH_MAX seem to be able to be bigger, and worse: we can't
be sure we're given longer args).
The code was rewrote to natively include boundary checks so we're
sure we won't overflow the buffer. A few strncpy have been removed
because in this case they tend to make things harder to write.
When a #include is used, the file is searched in many places; when
the file was searched in the ':'-separated list of path it could
led to infinite loop if the list contained more than one path and
that the file was not found in the first path, the ':' separator
was not properly passed over.
The Debian script for WindowMaker generates some warnings on startup
due to 'find' being cautious about its options, updated them to be
in line with expectations.
It helps to understand why a file is present in this file, which
in turn can help to understand where a file comes from and it makes
updates to the file easier
Now that the built-in parser has support for all the feature of CPP
being used by WindowMaker's default menu, we can remove the stuff
related to calling CPP:
- code for preparing and running CPP;
- compile-time option to de-activate the call to CPP;
- command-line option
It is now possible to use #ifdef/#ifndef to exclude some part of the
file. The implementation uses a stack to track conditionals so it is
possible to use nested constructs.
A number of macros are pre-defined by WindowMaker for CPP in the
function 'MakeCPPArgs', they are now available in the internal
parser too. CPP also had some predefined macros, a subset of them
have been added.
The definition have been split in two parts:
- the macro that are dependant on WindowMaker parameters are
defined by WindowMaker (src/rootmenu.c)
- those that are independant, which can be defined by the parser
itself (WINGs/menuparser_macros.c)
This adds support for defining new macros, with or without parameters, which
when found afterwards in the text are replaced by their definition.
The complex analysis for arguments replacement is done at macro definition
time, so it is done only once and the macro expansion will be fast.
The macro-related functions have been placed in their own file because it is
quite a complex task and we do not want filesize to explode, it is always
better to keep things human-sized.
The parser is prepared to handle '#' directives, starting with file
inclusion. The search path for the file are taken from what was
actually given to CPP. There is an arbitrary limit to the inclusion
nesting, which is actually not a design limitation but a security
to avoid infinite include loops.
Wrote a new parsing code that is able to skip over comments.
Note that because CPP is still active, you will not see any
effect unless you disable CPP pre-processing.
The default function used so far provides informations not so useful
to user, like wmaker's source file, line number and function; it
also cannot provide the line number from the parsed file because cpp
messes this information.
With this dedicated function we try to provide useful things which
are being tracked by the parser internally, like valid line number
and the name of the file being read (which can be convenient in the
case of #include, for which we may also be able to provide the
inclusion tree!)
From caller point of view, the two function have been merged into a
single function in the API. This will be needed by the advanced
parser that will have to not separate the concept of a 'line' and
the concept of 'content' (due to empty/comment lines, multi-line
comments, long lines split with '\')
All the information related to the file being parsed are stored in
a single place. The content of this structure is not visible to
caller to avoid messing the content; the parsing will be handled as
methods to this object.
Please note that all functions visible as part of the parser's API
are using the CamelCaseNotation to be consistent with the rest of
the API; however all internal functions use the non_camel_case_syntax
to follow the coding style set by Carlos for the project.
Due to the tasks to take in charge, the internal parser will grow in
size to support basic CPP feature, so it is a good idea to start by
moving the current functions into a dedicated file.
Christophe CURIS