[RFC] [PATCH 6/8] [libXRes] Implemented second part of XResource extension v1.2: XResQueryResourceBytes

[Tiago Vignatti]

On Mon, Dec 27, 2010 at 04:57:00PM +0200, ext Erkki Sepp�l� wrote:
> Reviewed-by: Rami Ylim??ki <rami.ylimaki at vincit.fi>
> ---
>  include/X11/extensions/XRes.h |   32 +++++++++++++
>  src/XRes.c                    |  102 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 134 insertions(+), 0 deletions(-)
> 
> diff --git a/include/X11/extensions/XRes.h b/include/X11/extensions/XRes.h
> index d674423..c16c417 100644
> --- a/include/X11/extensions/XRes.h
> +++ b/include/X11/extensions/XRes.h
> @@ -45,6 +45,24 @@ typedef struct {
>    void         *value;
>  } XResClientIdValue;
>  
> +typedef struct {
> +  XID           resource;
> +  Atom          type;
> +} XResResourceIdSpec;
> +
> +typedef struct {
> +  XResResourceIdSpec spec;
> +  long          bytes;
> +  long          ref_count;
> +  long          use_count;
> +} XResResourceSizeSpec;
> +
> +typedef struct {
> +  XResResourceSizeSpec  size;
> +  long                  num_cross_references;
> +  XResResourceSizeSpec *cross_references;
> +} XResResourceSizeValue;
> +
>  _XFUNCPROTOBEGIN
>  
>  /* v1.0 */
> @@ -100,6 +118,20 @@ void XResClientIdsDestroy (
>     XResClientIdValue  *client_ids
>  );
>  
> +Status XResQueryResourceBytes (
> +   Display            *dpy,
> +   XID                 client,
> +   long                num_specs,
> +   XResResourceIdSpec *resource_specs, /* in */
> +   long               *num_sizes,      /* out */
> +   XResResourceSizeValue **sizes        /* out */

nitpick here, but you could fix the indentation of this last comment.


> +);
> +
> +void XResResourceSizeValuesDestroy (
> +   long                num_sizes,
> +   XResResourceSizeValue *sizes
> +);
> +
>  _XFUNCPROTOEND
>  
>  #endif /* _XRES_H */
> diff --git a/src/XRes.c b/src/XRes.c
> index e64dc10..9dd0091 100644
> --- a/src/XRes.c
> +++ b/src/XRes.c
> @@ -341,3 +341,105 @@ pid_t XResGetClientPid(
>          return (pid_t) -1;
>      }
>  }
> +
> +static Status ReadResourceSizeSpec(
> +   Display               *dpy,
> +   XResResourceSizeSpec  *size
> +)
> +{
> +    _XRead32(dpy, &size->spec.resource, 4);
> +    _XRead32(dpy, &size->spec.type, 4);
> +    _XRead32(dpy, &size->bytes, 4);
> +    _XRead32(dpy, &size->ref_count, 4);
> +    _XRead32(dpy, &size->use_count, 4);
> +    return 0;
> +}
> +
> +static Status ReadResourceSizeValues(
> +   Display                *dpy, 
> +   long                    num_sizes, 
> +   XResResourceSizeValue  *sizes)
> +{
> +    int c;
> +    int d;
> +    for (c = 0; c < num_sizes; ++c) {
> +        CARD32 num;
> +        ReadResourceSizeSpec(dpy, &sizes[c].size);
> +        _XRead32(dpy, &num, 4);
> +        sizes[c].num_cross_references = num;
> +        sizes[c].cross_references = num ? calloc(num, sizeof(*sizes[c].cross_references)) : NULL;
> +        for (d = 0; d < num; ++d) {
> +            ReadResourceSizeSpec(dpy, &sizes[c].cross_references[d]);
> +        }
> +    }
> +    return Success;
> +}
> +
> +Status XResQueryResourceBytes (
> +   Display            *dpy,
> +   XID                 client,
> +   long                num_specs,
> +   XResResourceIdSpec *resource_specs, /* in */
> +   long               *num_sizes, /* out */
> +   XResResourceSizeValue **sizes /* out */
> +)
> +{
> +    XExtDisplayInfo *info = find_display (dpy);
> +    xXResQueryResourceBytesReq *req;
> +    xXResQueryResourceBytesReply rep;
> +    int c;
> +
> +    *num_sizes = 0;
> +
> +    XResCheckExtension (dpy, info, 0);
> +
> +    LockDisplay (dpy);
> +    GetReq (XResQueryResourceBytes, req);
> +    req->reqType = info->codes->major_opcode;
> +    req->XResReqType = X_XResQueryResourceBytes;
> +    req->length += num_specs * 2; /* 2 longs per client id spec */
> +    req->client = client;
> +    req->numSpecs = num_specs;
> +
> +    for (c = 0; c < num_specs; ++c) {
> +        Data32(dpy, &resource_specs[c].resource, 4);
> +        Data32(dpy, &resource_specs[c].type, 4);
> +    }
> +
> +    *num_sizes = 0;
> +    *sizes = NULL;
> +
> +    if (!_XReply (dpy, (xReply *) &rep, 0, xFalse)) {
> +        goto error;
> +    }
> +
> +    *sizes = calloc(rep.numSizes, sizeof(**sizes));
> +    *num_sizes = rep.numSizes;
> +
> +    if (ReadResourceSizeValues(dpy, *num_sizes, *sizes) != Success) {
> +        goto error;
> +    }
> +
> +    UnlockDisplay (dpy);
> +    SyncHandle ();
> +    return Success;
> +
> + error:
> +    XResResourceSizeValuesDestroy(*num_sizes, *sizes);
> +
> +    UnlockDisplay (dpy);
> +    SyncHandle ();
> +    return !Success;
> +}
> +
> +void XResResourceSizeValuesDestroy (
> +   long                num_sizes,
> +   XResResourceSizeValue *sizes
> +)
> +{
> +    int c;
> +    for (c = 0; c < num_sizes; ++c) {
> +        free(sizes[c].cross_references);
> +    }
> +    free(sizes);
> +}

In general this commit seems okay, but then again, it's difficult to review
without having the first one together:
http://gitorious.org/erkkise/libxres-xresources/commit/568f139caed330574951872d936002ca9627ce97  

So I'd ask to re-send all the patchset again with the whole changes between
what we have at fd.o upstream at this moment and the changes you are
introducing.

Another thing you can amend in the previous commit is reason of going from 1.0
to 1.2. Just a short commentary, like the one you mentioned in the
resourceproto, inside the XRes.h should be enough.

And you can work a bit on the commit subject also of these. Something like
"version 1.2: implement client identification (XResQueryClientsIds)" seems
more clear and easy to understand in my opinion.


Thanks,
             Tiago