[ofa-general] RE: [PATCH][3] opensm: per mesh node information

[Robert Pearson]

Hi Sasha

You wrote:

> +	if (!(node->links = calloc(num_ports, sizeof(link_t *))))
> +		goto err;
> +
> +	for (i = 0; i < num_ports; i++) {
> +		if (!(node->links[i] = calloc(1, sizeof(link_t))) ||
> +		    !(node->links[i]->ports = calloc(num_ports,
> sizeof(int))))
> +			goto err;
> +	}

Assuming that ports array is preallocated, wouldn't it be simpler to
define link as:

typedef struct _link {
	int switch_id;
	int link_id;
	int num_ports;
	int next_port;
	int ports[0];
} link_t;

, and then:

	node->links[i] = calloc(1, sizeof(link_t *) + num_ports *
sizeof(int))))

?

(Similar optimizations are probably relevant in other places).

I agree they accomplish the same goal. It is a tradeoff between code that is
a little shorter and faster and ease of understanding. I don't have strong
feelings. (For the same reason I tend to use 'x = calloc(1, foo)' instead of
'x = malloc(foo); memset(x, 0, foo);' which is a very common usage pattern.)

The same applies to your later note. We can represent a two dimensional
array as

int **array;

followed by array = calloc(1, n*sizeof(int *));
		array[i] = calloc(1, m*sizeof(int)); ...

and then you get to type

		array[i][j] = xxx;

vs

int *array;

array = calloc(1, m*n*sizeof(int));

and then

array[i*m+j] = xxx;

You can't use array[i][j] here because the compiler doesn't know the size of
the array until run time.


If the code is at all complex I prefer the [][] notation because it is
easier to read and understand. The optimizer in the compiler will take the
pointer dereference or the multiply out of inner loops so there is not
normally a big performance difference.

I guess that this code is complex enough that at least for now it is
preferable to err on the side of keeping everything as straight forward as
possible until we are sure that it is correct. Then if performance is an
issue we can optimize it.

I am happy either way. Let me know what you want me to do.

Regards,

Bob