connector.h

connector.h

[Andrew Morton]

> 
> struct cb_id
> {
> 	__u32			idx;
> 	__u32			val;
> };

It is vital that all data structures be skilfully commented - they are the
key to understanding the code.  Why the struct exists, which actor passes
it to which other actor(s), whether the data structure is communicated with
userspace, what other data structures it is aggregated with or linked to,
locking rules, etc.

> struct cn_msg
> {

Please do

	struct cn_msg {

> 
> #define CN_CBQ_NAMELEN		32

Commentary?

Re: connector.h

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On Thu, 2005-03-31 at 17:31 -0800, Andrew Morton wrote:
> > 
> > struct cb_id
> > {
> > 	__u32			idx;
> > 	__u32			val;
> > };
> 
> It is vital that all data structures be skilfully commented - they are the
> key to understanding the code.  Why the struct exists, which actor passes
> it to which other actor(s), whether the data structure is communicated with
> userspace, what other data structures it is aggregated with or linked to,
> locking rules, etc.

It is described in Documentation/connector/connector.txt.
Should it also be placed here?

> > struct cn_msg
> > {
> 
> Please do
>
> 	struct cn_msg {

Neither structure declaration should have opening brace on the new
string?

> > 
> > #define CN_CBQ_NAMELEN		32
> 
> Commentary?

Maximum allowed callback name - name will be truncated if 
it exceeds that limit.

I will place this doc in the code.

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: connector.h

[Andrew Morton]

Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:
>
> On Thu, 2005-03-31 at 17:31 -0800, Andrew Morton wrote:
> > > 
> > > struct cb_id
> > > {
> > > 	__u32			idx;
> > > 	__u32			val;
> > > };
> > 
> > It is vital that all data structures be skilfully commented - they are the
> > key to understanding the code.  Why the struct exists, which actor passes
> > it to which other actor(s), whether the data structure is communicated with
> > userspace, what other data structures it is aggregated with or linked to,
> > locking rules, etc.
> 
> It is described in Documentation/connector/connector.txt.
> Should it also be placed here?

I think it's better to document these things in the code.  Those structs
which are communicated to userspace should be described in connector.txt
because they are part of the API.  But a lot of the structs you have there
are purely knerel-internal.

> > > struct cn_msg
> > > {
> > 
> > Please do
> >
> > 	struct cn_msg {
> 
> Neither structure declaration should have opening brace on the new
> string?

I don't understand your question.

We lay out struct definitions thusly:

struct foo {
	int a;
	int b;
};