>>> om_getDiskPartitionInfo: why not just take a DiskInfo as the first
>>> argument? And what is meant by "A failure will be fatal in this case"?
>> The DiskInfo structure returned from om_getDiskInfo is a linked list of
>> DiskInfo. Why the whole structure needs to be passed back when diskName
>>is enough to get the partitions.
>>

>If this were the long-term design I'd be pushing strongly to have the
>callers not directly referencing structure members but instead using
>accessor functions, because that gives us much more flexibility in
>evolving the design, so generally I'm suggesting that callers not have
>to grovel around in the structures to use results from one call to make
>another call.

+1 on the accessor functions suggestion. I'm even considering writing my
own wrapper functions in the GUI rather than randomly poking at data
structures.
There's something missing from this function's prototype or the data structures
of it's parameters:
 - How can you make a linked list of DiskInfo_t types? There is no "next" member
   in the DiskInfo_t type to point to the next one. It seems like a more 
intuitive 
   approach would be to migrate this function to a DiskTarget based operation 
and
   add a seperate DiskInfo member accessor function:

int om_getDiskTargets (DiskTarget_*t dtargets, int *total);
    which would return OM_SUCCESS or OM_NODISKS_FOUND. dtargets would be a 
    a pointer to a DiskTarget_t type linked list, allocated on the caller's 
behalf, and total
    would be set to the number of DiskTarget_t types found.

The caller could then get at the DiskInfo structure data by accessing inside 
the desired
DiskTarget structure. Or perhaps an accessor/convenience function :) :
    DiskInfo_t* om_DiskTarget_getDiskInfo (DiskTarget_t *dtarget); or...
    DiskInfo_t* om_disk_target_get_disk_info (DisktTarget_t *dtarget); <-- 
cleaner namespacing :)

    Since the DiskTarget_t linked list was already allocated on the caller's 
behalf by
    om_getDiskTargets (), no need to allocate another copy.

Cheers,
Niall.
--
This message posted from opensolaris.org

Reply via email to