[wmii] libixp

Hello,

yesterday I went through the libixp and wmii sources and created patches
which introduce the use of union within libixp's Fcall structure again,
but use named union and structs.

However, there are some code-style issues arising. At some points the
code is ambiguous on whether the handled data belongs e.g. to a Ropen,
Rcreate or Rattach. At other points the code is optimized to handle
e.g. all three cases at once (as the handling is the same). But this
sharing of common handling is not very systematic, e.g. not symmetric
with respect to (Ropen,Rcreate,Rattach) and (Rauth) but (Tauth,Tattach)
and (Topen,Tcreate) (the parentheses denote the sharing of the same
structure).
This problem does not show up with the anonymous structs, but is there
and quite ugly.
Also, the use of anonymous structs lead to a variety of names, which
represent the same content in a different structure (e.g. Qid qid in
Ropen, Rcreate, Rattach and Qid aqid in Rauth) and make the code ugly
and harder to read.
Moreover, consider this piece of code in client.c:

        if(filepath) {
                c->ifcall.name = filepath;
                c->ifcall.nwname = ixp_tokenize(wname, IXP_MAX_WELEM, c->ifcall.name, '/');
                for(i = 0; i < c->ifcall.nwname; i++)
                        c->ifcall.wname[i] = wname[i];

Here name, nwname and wname are used at the same time, though they belong
to different structs within the union. And looking at the corresponding
excerpt from ixp.h (occurring within a union):

                struct { /* Tcreate */
                        unsigned int perm;
                        char *name;
                        unsigned char mode; /* +Topen */
                };
                struct { /* Twalk */
                        unsigned int newfid;
                        unsigned short nwname;
                        char *wname[IXP_MAX_WELEM];

                };

the char* name and unsigned short nwname may overlap!
With the use of named union and structs this would have been an obvious
mistake (using data.tcreate.name and data.twalk.nwname, knowing that
data is a union).

The code also contains some other quirks which are not good programming
style and should be revisited. Two things I came across are the modification
of arguments by called functions (e.g. filepaths, which I would expect to be
const as this is not a return value) and the freeing of memory as well.
If I allocate memory and pass a pointer to it to a function, I should take
care of freeing that memory afterwards, not the called function. Of course,
the other way round (the function allocates memory for a return value),
there's no other means then me freeing it after using the data.

Anyways, the patches make use of the union again and the code works on my
PC and my Sun...

Regards,
Stefan

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer