On Wed, Mar 01, 2017 at 11:54:20AM -0500, Frediano Ziglio wrote: > > > > On Tue, Feb 28, 2017 at 03:20:09PM +0000, Frediano Ziglio wrote: > > > Cursor resources (basically the shape of it) was retained till > > > it was used however it was copied so there were no reason to not release > > > this resource. > > > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > > --- > > > server/cursor-channel.c | 14 ++++---------- > > > server/cursor-channel.h | 4 ++-- > > > server/red-parse-qxl.c | 10 +++++++--- > > > server/red-parse-qxl.h | 4 ++-- > > > server/red-worker.c | 5 +++-- > > > server/tests/test-qxl-parsing.c | 26 +++++++++++++++++++++++--- > > > 6 files changed, 41 insertions(+), 22 deletions(-) > > > > > > diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c > > > index 89cb120..5ed36df 100644 > > > --- a/server/red-parse-qxl.c > > > +++ b/server/red-parse-qxl.c > > > @@ -27,6 +27,7 @@ > > > #include "red-common.h" > > > #include "memslot.h" > > > #include "red-parse-qxl.h" > > > +#include "red-qxl.h" > > > > > > /* Max size in bytes for any data field used in a QXL command. > > > * This will for example be useful to prevent the guest from saturating > > > the > > > @@ -1470,8 +1471,10 @@ static void red_put_cursor(SpiceCursor *red) > > > } > > > > > > int red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id, > > > - RedCursorCmd *red, QXLPHYSICAL addr) > > > + RedCursorCmd *red, QXLPHYSICAL addr, > > > + QXLInstance *qxl_instance) > > > > Not convinced about this patch, mainly I think because of the added > > QXLInstance dependency which gets added to red-parse-qxl.h, > > red_get_cursor_cmd() becoming odd compared to the other similar > > functions, ... > > > > I think it would work to have the red_qxl_release_resource() call in > > red_process_cursor_cmd()? > > > > This would be just a step before this... > Memory management and resources are two sides of the coin. > red-parse-qxl.c already deals with release_info information so > adding a free is not really a big jump. > Actually already some structures in red-parse-qxl.h has a QXLInstance > which is initialized by red-worker.c. Maybe this is the issue, some > structures are basically initialized half in red-parse-qxl.c and > half in red-worker.c. Dunno, but all the red_get_*_cmd() methods have a similar prototype, and don't have a QXLInstance argument, changing this just for red_get_cursor_cmd() makes it odd. Wrapping the QXLInstance together with the command to release it more easily might make sense. Looking through my old branches, I discovered some unfinished work doing that /o\ Guess it's time to rebase it ;) I'd prefer that we stick to the "step before this" (ie my suggestion) until we can make this more consistent in red-parse-qxl.h Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel