Re: Generic HDLC interface continued

Francois Romieu (romieu@cogenit.fr)
Sat, 28 Sep 2002 20:21:38 +0200


<disclaimer>
It's ok for me to trim the Cc: list if noboy objects.
Don't hesistate to ask for clarification if my english comes from Mars.
</disclaimer>

Krzysztof Halasa <khc@pm.waw.pl> :
[...]
> Addressing the second problem (unknown data length) requires the caller
> (user-space utils) to supply allocated space size. The kernel would
> update the size to reflect the actual amount of data required, allowing
> the caller to allocate more space and try again (or ignore the unknown
> interface).

If size/limit of an underlying object is in a structure for other purpose
than debygging, it means something (S) is working with an object and it (S)
doesn't know what it is. Design proble: always work with an object whose
identity you know or simply pass a reference to someone knowing better.

I don't see why a 'size' parameter is required. Thus I'm fine with the
following (we are lucky enough that there is enough space in union ifr_ifru
to hold ifru_settings):

struct ifreq
{
#define IFHWADDRLEN 6
#define IFNAMSIZ 16
union
{
char ifrn_name[IFNAMSIZ]; /* if name, e.g. "en0" */
} ifr_ifrn;

union {
struct sockaddr ifru_addr;
struct sockaddr ifru_dstaddr;
struct sockaddr ifru_broadaddr;
struct sockaddr ifru_netmask;
struct sockaddr ifru_hwaddr;
short ifru_flags;
int ifru_ivalue;
int ifru_mtu;
struct ifmap ifru_map;
char ifru_slave[IFNAMSIZ]; /* Just fits the size */
char ifru_newname[IFNAMSIZ];
char * ifru_data;
struct {
int type;
union {
raw_hdlc_proto *;
...
sync_serial_settings *;
etc_line_settings *;
}
} ifru_settings;
} ifr_ifru;
};

Note however that struct ifreq on amphetamin (wrt lines of code) doesn't
improve readability for everybody. That's a slightly different problem.

> What is important here is that inner union consists of pointers
> to *_proto / *_settings structs and not of the structs themselves.

Agree on this.

> Another solution - using a different ifreq structs for different tasks
> (something like the sockaddr_*) - sort of:
[...]

I am not too fond of this and, again, what are these 'size' for ?
If it's supposed to replace/duplicate ifreq, the 'settings' part should
be a pointer imho. Same reason as before: size change => compatibility
problems for tools (we have sources but downgrading tools when returning to
previous kernel sucks).

-- 
Ueimor
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/