There is a dedicaded floating point modulo operator that does a
better job (faster, no precision loss), so we use it instead of the
complicated formula that uses type conversions.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
For this case we happen to already have the same variable as integer
type, so it is better to use them directly for computation.
Please note that the floating point variables do not contain more
precision in this use case, otherwise the change would not be
transparent.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Compilation will not work if the header is missing, so make sure
it is present and compilable first. This can highligh sooner if
a user forgot to install the corresponding dev package.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Took the opportunity to rewrite the check using autoconf macros
to generate a more compatible configure script.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The function contained an internal statement that would have
discarded the const attribute, but as this statement is actually
useless it have been simply removed.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Because we have to use some X functions marked as deprecated to
ensure portability, we prefer to hide the warnings from standard
users, and keep them only for dev team.
[crm: Add -Wno-deprecated-declarations too]
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
To ensure proper behaviour with X servers that do not support the
Xkb extension, implemented call to the fall-back legacy function
to the other places where it is being used, to ensure proper
behaviour in any case.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The original code was assuming that the Xkb extension is available
everywhere, which is not the case. This resulted in all modifiers
to be seen as NoSymbol, thus the empty list.
Now we initialize properly the Xkb extension at the start of the
program, and provide a work-around with the historical function in
the case the ext would not be available.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Apparently, there is some exceptional case when the 'Mouse Grab
Modifier' list can be empty, in which case the program would
crash.
This patch makes sure the program will not crash (that's a bad
behaviour), but does not solve the empty list case
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As reported by clang, the format used was %hi which is expecting
a short-typed argument, but the parameters are actually defined
as ints in the structure, so use only %i.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
As reported by Iain Patterson, the clang compiler is (by default)
strictier on having clean C syntax. A few structure definition
did not comply, now they do.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
Using an unspecified prototype function for a callback is calling
for problem. Added an explicit prototype for the 2 callback
functions in WDefaultEntry, and fixed the functions used to match
it. The new code should be a lot safer.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
This attribute for functions was not standard until recently, so
there are a few different possible syntax. With this patch, the
configure script will search for the proper syntax, and define what
is needed to have the attribute compile correctly.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
If the function was called more than once with different usernames
it would always return the path for the user on the first call,
which is not what would be expected.
Furthermore, if the function succeeds it allocated memory to save
this path but it was never freed.
The good thing is that the use case for this function is so rare
that it is improbable it was ever called, which explains why it
was never seen.
The new code always behaves as expected, and does not allocate
memory anymore to avoid wasting time and memory for such small
things, which is acceptable because this function is local.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
The previous method was to use a custom install procedure, which
is a bit complex when automake can handle this for us, and as a
side effect it made 'distcheck' fail on uninstall procedure check.
The new method is to use a simple conditional and autoconf/automake
will do all the work for us.
Signed-off-by: Christophe CURIS <christophe.curis@free.fr>
- remove extern declaration in source file, use header instead
- add inclusion of header defining the functions of the file to
get the compiler to cross-check them
- marked static the functions that should not be visible ouside
their file
- remove extern declaration in source file, use header instead
- add inclusion of header defining the functions of the file to
get the compiler to cross-check them
- marked static the functions that should not be visible ouside
their file
This allows the compiler to warn if the definition in the file is
no in line with what is exposed to the users of the function through
the header definition.
The declarations have been split by source file, adding the usual
separation mark. Removed these 2 prototypes:
- W_SetFocusOfToplevel: case typo, the correct prototype already existed
in the file;
- W_TextWidth: function is not defined anywhere
When a function is used as a call-back, it is dangerous to have
arguments with a type different than what is expected by the
call-back definition.
This patch sets the argument list to match what is expected by
the call-back prototype and inserts an explicit type conversion
at the beginning of the function.
It is dangerous to let the compiler know about a function without
letting him know the arguments because he won't be able to report
invalid calls.
This patch concern the cases where adding the arguments led to
problems because the functions were used as call-back. As it is
dangerous to have parameter mismatchs in call-back, setup the
args as expected by prototype and added explicit conversion inside
the concerned function, so the compiler will know and be able to
do what may be necessary.
It is dangerous to let the compiler know about a function without
letting him know the arguments because he won't be able to report
invalid calls.
This patch concern the cases where adding the arguments did not
need other code change.
This is the correct way to tell that a function takes no
arguments, because an empty parameter list tells the compiler
that it is not yet defined, and is tolerated only for
compatibility with very old C compilers for whom prototypes
were not yet a defined language element.
The functions are now grouped by source file (groups in alphabetic
order) with the traditional separation mark. This makes the file
easier to work with.
It is not a good idea to multiply the number of header files,
specially in this case where 'double.h' defined so few things
(and lacked the usual copyright notice / include guards).
We probably don't want our users to have to endure them, so they
are enabled only when Debug is activated, because they tend to
help keeping the code safe.
The equality comparison (a == b) is known to be a dangerous trap
when floating-point arithmetics are involved. In the current case
the offending operation can be done with integers directly.
The equality comparison (a == b) is known to be a dangerous trap
when floating-point arithmetics are involved. This patch changes
all the cases which try to do this to a safer check.
The original code allowed to have 0.0, but this can generate
division by zero in WScrollView. As a value of 0.0 is not realistic
anyway, use a minimum constant instead.
The equality comparison (a == b) is known to be a dangerous trap
when floating-point arithmetics are involved. This patch changes
all the cases which try to do this to a safer check.
Some header were creating variable, this is a bad practice which
is likely to not behave as expected. This creates one distinct
variable in each object file that used the header, and:
- on well behaved compiler, this ends up in a link error (see
commit 39fdb451ba for an example)
- on bad behaving compiler, this can be linked as multiple local
variable, thus having strange effects when running program
- on insouciant compiler (who said gcc?) the variables are
silently merged, hiding portability issues
Autoconf provides the necessary stuff to detect if inline keyword
is supported, and to detect special syntaxes, so let's use this
and remove the multiple local definitions, this makes code simpler.
When using the formula [sizeof(array) / sizeof( x )] to get the number
of element in a static array, it is better to use array[0] for 'x'
instead of the base type of array:
- in case the base type would change someday;
- if the compiler were deciding to insert padding somewhere
Some compilation options are actually targetting the preprocessor
instead of the compiler; using the wrong variable can have some
subtile side effects, so let's get things right.
Considering the number of headers we have, it is a good idea to
avoid possible problems. For details, you may read:
http://en.wikipedia.org/wiki/Include_guard
All headers should be ok now.
The automake documentation states that using substitution inside the
list of SOURCES will not work and calls for not doing it. The use
of 'EXTRA_xxx' made things look like they worked but is probably not
enough for corner cases.
This patches switches to the conditional method which will be safe.
To be able to do this in a clean way, it was necessary to add the
attribute also in PLData's ptr field, which is actually right
because none of the function changes its content.
The function that fills it from a file/pipe however needed small
changes to respect the const-ness of the field.
The history is actually loaded from a file into a property list that
is then converted to an array. The intermediate property list was
not freed, which led to memory leak.
It looks like it was a tentative of optimisation to avoid duplicating
an already allocated string and re-use the pointer instead, but this
means it is not possible to free the original container as it would
free the string too.
There is a better way to do this, but it requires an API change on the
WUtil library so it is left for a future improvment.
The previous code limited the number of entries that were read into
the history array, but the user is expecting a maximum on the
number of entries displayed. This can make a little difference in
two cases:
- if there are duplicate entries (dups are checked for and removed)
- if some entries are not strings (unlikely, but not impossible)
The new code just stops adding history entries when the user
specified count is reached.
When menus are read in the PropList format, they are loaded into a
temporary PropList object, which is parsed into the internal menu
structure, and the PropList object is no more used. There were two
cases where this temp object was not freed.
A common argument to all these functions is the name of the key to
operate on, and this name is never modified by the functions. Marking
it as const reflects this, and can allow compiler to generate better
results thanks to this info.
The addition of the const attributes is actually an API change, so
we have to reflect this for the next official release.
Because the change on 'wusergnusteppath' may impact users of the API,
we won't only change REVISION like it was done for WRaster lib.
The first optimisation is to compute only once the path, and then
always re-use the value which did not change anyway.
The second optimisation is to avoid a lot of excessive function
calls, including alloc+free that are not necessary and participate
in memory fragmentation.
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.
Christophe CURIS