Re: Fear and loathing in ipcname_gettotal.

Roger Tsang <[email protected]> Sat, 28 Mar 2009 20:10:12 +0000
Newsgroups gmane.linux.cluster.ssic.devel
Message-ID <[email protected]>
Looks good.  I've cleaned up your patch (K&R style) for the next CVS checkin.

2009/3/26 John Hughes <[email protected]>:
> Here is a patch to clean up the ipcname_gettotal ugliness.
>
> After this patch it is the job of callers of ipcname_gettotal (or
> cli_ipcname_gettotal) to allocate the memory.  We avoid the memory leak
> where a caller allocates the memory then calls ipcname_gettotal that
> reallocates it.
>
> We also pass the (node, key) lists around as a nice struct instead of some
> int's bashed into an array of chars.
>
> Before in ipc/shm.c:
>
>       if (!local_view) {
>               size = -1;
>               cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
>               if (size <= 0) goto done;
>               node_id_pairs = (char *)kmalloc(size * sizeof(int),
> GFP_KERNEL);
>               if (node_id_pairs == NULL)
>                       goto done;
>               memset(node_id_pairs, 0, size * sizeof(int));
> cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs, &size);
>               tmp_pairs = (char *)node_id_pairs;
>       }
>       id_count = SHM_MAX_ID;
>
>       for(i = 0; i <= id_count; i++) {
>               struct shmid_kernel* shp;
>               shp = NULL;
>               if (!local_view) {
>                       node_num = *((int *)tmp_pairs);
>                       tmp_pairs += sizeof(int);
>                       ipc_id = *((key_t *)tmp_pairs);
>                       tmp_pairs += sizeof(key_t);
>                       i++;
>
> (Notice that sneaky little i++ there?  It's because ipcname_gettotal returns
> the number of int's, not the number of (node,key) pairs).
>
> After:
>
>       if (!local_view) {
>               size = 30;      /* Random guess */
>               for (;;) {
>                       int allocated = size;
>                       node_id_pairs = kmalloc(size * sizeof *node_id_pairs,
> GFP_KERNEL);
>                       if (!node_id_pairs) goto done;
>                       cli_ipcname_gettotal(NAME_SERVICE_SHM, &node_id_pairs,
> &size);
>                       if (size <= allocated) break;
>                       kfree (node_id_pairs);
>               }
>       }
>       id_count = SHM_MAX_ID;
>
>       for(i = 0; i <= id_count; i++) {
>               struct shmid_kernel* shp;
>               key_t ipc_id = i;
>               int segsize=0, cprid=0;
>               shp = NULL;
>               if (!local_view) {
>                       ipc_id = node_id_pairs[i].ipc_id;
>                       node_num = node_id_pairs[i].node_num;
>
>
>
> ------------------------------------------------------------------------------
>
> _______________________________________________
> ssic-linux-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ssic-linux-devel
>
>

------------------------------------------------------------------------------