[PATCH][weston] animation: Fix potential leak of memory pointed to by move

[Lucas Tanure]

Free move before return if animation is null.

Signed-off-by: Lucas Tanure 
---
 src/animation.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/animation.c b/src/animation.c
index cc7482d..2c7943f 100644
--- a/src/animation.c
+++ b/src/animation.c
@@ -471,8 +471,10 @@ weston_move_scale_run(struct weston_view *view, int dx, 
int dy,
animation = weston_view_animation_create(view, start, end, move_frame,
 NULL, move_done, data, move);
 
-   if (animation == NULL)
+   if (animation == NULL){
+   free(move);
return NULL;
+   }
 
weston_spring_init(>spring, 400.0, 0.0, 1.0);
animation->spring.friction = 1150;
-- 
2.6.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston 1/2] compositor: don't crash if destroying a compositor without a backend

[Giulio Camuffo]

Calling weston_compositor_destroy() on a pointer returned by
weston_compositor_create() should be always valid, even if the
compositor does not have yet a backend.

Signed-off-by: Giulio Camuffo 
---
 src/compositor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/compositor.c b/src/compositor.c
index 125afd5..f8437e8 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -4767,7 +4767,8 @@ weston_compositor_destroy(struct weston_compositor 
*compositor)
 
weston_compositor_xkb_destroy(compositor);
 
-   compositor->backend->destroy(compositor);
+   if (compositor->backend)
+   compositor->backend->destroy(compositor);
free(compositor);
 }
 
-- 
2.6.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston 2/2] main: fix destroying the compositor on error while starting

[Giulio Camuffo]

Signed-off-by: Giulio Camuffo 
---
 src/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/main.c b/src/main.c
index a98570e..1591018 100644
--- a/src/main.c
+++ b/src/main.c
@@ -730,16 +730,16 @@ int main(int argc, char *argv[])
ec = weston_compositor_create(display, NULL);
if (ec == NULL) {
weston_log("fatal: failed to create compositor\n");
-   goto out_signals;
+   goto out;
}
 
ec->config = config;
if (weston_compositor_init_config(ec, config) < 0)
-   goto out_signals;
+   goto out;
 
if (backend_init(ec, , argv, config) < 0) {
weston_log("fatal: failed to create compositor backend\n");
-   goto out_signals;
+   goto out;
}
 
catch_signals();
-- 
2.6.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Nils Chr. Brause]

Hi,

Pekka Paalanen wrote:
> Adding these attributes to XML must not change the signatures generated
> by wayland-scanner.
>
> Why: these are also used in C++, which does not implicitly covert
> between enums and other stuff, which would break the build.
> http://lists.freedesktop.org/archives/wayland-devel/2014-September/017057.html

There are two different types of enumerations in C++11. The C-style
'enum', which behaves in C++ just like it did in C and the modern
'enum class', which makes each enum a distinct type. The
wayland-scanner generates C code and therefore C-style enums. In C++
they are still nothing else than integers (just like in C). Therefore
they don't change any signatures.

Pekka Paalanen wrote:
> Later that thread also brought up two incompatible definitions of a
> "bitfield":
> - a type where bit-wise logic operations make sense
> - a type where all listed values are powers of two (POT), and bit-wise
>   logic operations make sense
>
> As a practical example, is wl_output::transform a bitfield or an enum?
> - it is used as an int, not uint
> - it lists all possible values
> - the listed values are not all POT
> - bit-wise operations make sense (check for flipped)
>
> Arguably it should have been an uint, but I don't think we can change
> it now.

I have a rather pragmatic view at this: If it looks and feels like a
bitfield, it is a bitfield.
And when looking an the definition, it for all the wold looks like a
bitfield to me.
Bit 0 means 90°, bit 1 means 180° and bit 3 means flipped.
The signedness doesn't matter for a bitfield, since it is just a
collection of bits without a numerical meaning.

Pekka Paalanen wrote:
> There is a paradox: if the new attributes affect codegen for some
> languages in a way that will break the build (which is exactly the
> reason why they are wanted: to enforce type-safety), we cannot add
> these attributes to the existing protocols, most importantly
> wayland.xml, because it would "break existing code".

If the correct types have already been used before, the changes
shouldn't actually break any code. If they do, that means that the
code was invalid in the first place (e.g. enum X used where enum Y is
required orr ORing together non-bitfield values).

Pekka Paalanen wrote:
> Should we make any stability guarantees towards non-C bindings
> generators that exceed what we need for C?

It should at least be guaranteed, the the enum and bitfield attributes
are correct. And that can ultimatly only done by a human. The scanner
might be able to detect if a specified enum actually exists, but only
a human can tell if it is the correct one.

Pekka Paalanen wrote:
> yeah, adding the interface name makes perfect sense. It could be
> optional if wanted. No dot would mean the enum is in the same
> interface, a dot would signify a specific interface. I don't think
> there is any use for an anonymous global namespace like Bill mentioned.

Sounds good to me. Although I think it would be easier to implement,
if we'd just use the dot notation everywhere. This would result in
less code and (statistically) less bugs.

Auke Booij wrote:
> The enum and bitfield attributes are in principle for documentation
> purposes only.

No. The whole point of the enum an bitfield attributes is to help
non-C languages to generate type-safe bindings.

Auke Booij wrote:
> The enum and bitfield attributes may also be used by
> bindings, but only in such a way that code written prior to the
> specification of these attributes still works after their
> specification.

I think every language binding should ultimately decide for
themselves, how they are using the new attributes. There is no way you
can dictate anyone how to use them.

Pekka Paalanen wrote:
> We could have a "strict mode" to wayland-scanner, where you need to
> feed all relevant additional XML files also, and the check would be
> cross-file then, erroring on all unknown links. Then we'd just change
> our builds to use this mode.

This would be very useful to to detect misspellings in interface names.

Bryce Harrington wrote:
> Sounds like might be something worth adding to distcheck.

Absolutely!

Auke Booij wrote:
> I agree with your concern. However, note that "for documentation
> purposes" does not mean that it is just a documentation string: we
> document a very clear and precise fact, for the purpose of
> understanding the API (ie documentation). My previous patches indeed
> do some checking wrt enum name cross-matching. This would have to be
> extended for enum names that refer to a different interface.

You shouldn't write "for documentation purposes only" then, as it is
clearly not. It is mainly for the type-safety of non-C
language-bindings.

Cheers,
Nils
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: Enums, bitfields and wl_arrays

[Erik De Rijcke]

The enum discussion seems to become a lengthy one and it's becoming hard to
see the forest for the trees.

I'll give my input as a Java bindings dev. Please correct any bad
assumptions or observations I've made.

>From what I see in Auke's patches:

- Defining an uint (bitfield) or int (single enum) in the xml for an enum
argument is the wrong approach. It prescribes, not describes. In some
languages eg Java, there is no such thing as an uint. What is important, is
to know how many enum values you are allowed to pass in an enum argument.
How this translates low level is an implementation detail (wire format). We
don't define opcodes either. The wire format in then end, is implicitly
defined by the C generator based on the argument type in the xml (for an
enum arg, some cases use an array of shorts, some cases use a int
bitfield.). It would be better to define that relationship.

General concerns as a language binding dev:

- The definition of an enum, at least in Java (and I assume in a lot of
other languages too), is a fixed set of values that are know at compile
time. This set is unmodifiable at runtime. This means no new values of an
enum type can be (safely) added/defined at runtime. It seems to me this
definition is ultimately incompatible with wayland enums for several
reasons.

First reason is the case of an 'open' enum, as seen in xdg-shell state.
>From my understanding, a client or server should be able to act on any
'open' enum value it receives over the wire, even if this value was not
known at compile time. As such a generated binding can not simply map all
unknown wire values to the same "UNKNOWN" language specific enum type as
the raw value would be lost in such case. It also makes it impossible to
send out any undefined raw value. An 'open' enum is thus impossible to
implement using a (language specific) enum as type. The core reason being
that a wayland 'open' enum is not really an enum but a set of pre-defined
constants.

The second reason is the case of the 'closed' enum, as seen in all other
enum definitions. A 'closed' wayland enum does allow to be directly mapped
to a language specific enum type. However there is still the case of enum
values known only by the other side (=different protocol versions). Again
the solution(s) for this are the same as the first reason. Map to an
'UNKNOWN" value and have your raw value lost, or pass on the raw value but
loose your language enum type usage.

Both concerns seem to have no relevance to wayland per se, and should
simply be solved by the language binding. I believe this not to be the case
as there is the open question (for me) on what should be considered as
acceptable behaviour when dealing with wayland enums in language bindings.
If it is acceptable to have your raw value lost for a 'closed' enum, but
not for an open value, then the language binding must be able to deal with
that appropriately. However for that it most know in some way when an enum
is open, and when it is closed (either by implicit ruling, eg it's an array
thus open, or explicitly through an xml attribute).

amen

On Thu, Oct 1, 2015 at 7:49 PM, Bryce Harrington 
wrote:

> The topic of adding better enum/bitfield support to the protocol has
> come up a few[0] times[1] in the past, and again more recently[2].  We
> also have several proposed patches in patchwork, but I'm not sure they
> reflect consensus and none have Reviewed-by's on them yet [3,4,5,6,7].
>
> From what I gather of the discussions, no one is really against this
> feature conceptually, and impementationally the discussions appear to
> have moved further afield.  It feels like we're real close to having
> something that could be landed, but it's not 100% clear to me what
> exactly to land.  Since it's a protocol types change I would prefer to
> make sure it has a strong consensus before landing it.
>
> I know that several people have proposed patches on this - Bill, Nils
> and Auke at least.  Since there's a definite need for this, and since
> agreement appears to be not far off, I would like to get this landed
> this release.  And ideally I'd like to get this landed early in the
> release so we give plenty of time for testing.
>
> Since Auke's patchset proposalis the most recent, let's take that one as
> the candidate for landing.  Gentlemen, I'd like to ask you to review
> these three patches [5,6,7] and either give your Reviewed-by's or flag
> specific improvements needed.  If you have a more conceptual
> disagreement, and don't think the patchset is landable as implemented,
> please raise that issue asap too.
>
> Bryce
>
> 0:
> http://lists.freedesktop.org/archives/wayland-devel/2015-April/021438.html
> 1:
> http://lists.freedesktop.org/archives/wayland-devel/2015-June/023008.html
> 2:
> http://lists.freedesktop.org/archives/wayland-devel/2015-September/024249.html
>
> 3: http://patchwork.freedesktop.org/patch/47726/
> 4: http://patchwork.freedesktop.org/patch/47727/
> 5: