On 10 January 2012 09:44, Alon Levy <al...@redhat.com> wrote:
> All but the last are assigned but unused variables,
> and an always true comparison, but the last looks like a logic
> error - decode not returning the actual return value for the last call
> in the buffer (vmgl-exec.c).
> ---
> gl/gloffscreen-xcomposite.c | 4 +---
> gl/vmgl-exec.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/gl/gloffscreen-xcomposite.c b/gl/gloffscreen-xcomposite.c
> index 5c63ed4..00a419b 100644
> --- a/gl/gloffscreen-xcomposite.c
> +++ b/gl/gloffscreen-xcomposite.c
> @@ -186,8 +186,6 @@ static void glo_test_readback_methods(void)
> /* Initialise gloffscreen */
> int glo_init(void)
> {
> - XErrorHandler old_handler;
> -
> if (glo_inited) {
> return 0;
> }
> @@ -204,7 +202,7 @@ int glo_init(void)
> /* Safe because we are single threaded. Otherwise we cause recursion
> * on the next call. Set the X error handler. */
> glo_inited = 1;
> - old_handler = XSetErrorHandler(x_errhandler);
> + (void)XSetErrorHandler(x_errhandler);
> glo_test_readback_methods();
>
> return 0;
> diff --git a/gl/vmgl-exec.c b/gl/vmgl-exec.c
> index e538ff4..3cf5eba 100644
> --- a/gl/vmgl-exec.c
> +++ b/gl/vmgl-exec.c
> @@ -668,7 +668,7 @@ static int glXGetConfigFunc(uint32_t visualid, uint32_t
> attrib, uint32_t *value)
> {
> const VmglGLXFBConfig *config = &fbconfigs[0];
>
> - if (visualid >= 0 && visualid < ARRAY_SIZE(fbconfigs)) {
> + if (visualid < ARRAY_SIZE(fbconfigs)) {
> config = &fbconfigs[visualid];
> } else {
> DEBUGF("Unknown visual ID %d\n", visualid);
> @@ -3072,11 +3072,12 @@ static inline int do_decode_call(ProcessStruct *p,
> const uint8_t *args_in,
> int args_len, uint8_t *r_buffer)
> {
> Signature *signature;
> - int i, ret;
> - const uint8_t *arg_ptr, *tmp;
> + int i;
> + const uint8_t *arg_ptr;
> static arg_t args[50];
> int func_number;
> ProcessState *process = DO_UPCAST(ProcessState, p, p);
> + int ret = 1;
>
> if (!args_len) {
> return 0;
> @@ -3099,7 +3100,6 @@ static inline int do_decode_call(ProcessStruct *p,
> const uint8_t *args_in,
> #endif
>
> signature = (Signature *) tab_opengl_calls[func_number];
> - tmp = arg_ptr;
>
> for (i = 0; i < signature->nb_args; i++) {
> int args_size = *(const uint32_t *) arg_ptr;
> @@ -3220,7 +3220,7 @@ static inline int do_decode_call(ProcessStruct *p,
> const uint8_t *args_in,
> }
> #endif
>
> - return 1;
> + return ret;
Thanks for the compile test. I think I was meaning to check ret after
every do_function_call call and return immediately if 0. I'll fix it
in a future iteration and add your other fixes if you don't mind.
Cheers
