Max and Cecilia, This case looks good overall; I'd like a few clarifications and have a few questions/comments and nit-picks in-line:
On Tue, 2008-08-12 at 15:40 +0800, Cecilia Hu wrote: > 4.2. Python 'dlpi' module > > Classification of interfaces provided in this module is 'Committed'. > The on-line help clearly shows all interfaces and their counterparts > of dlpi functions in this module as below: In documenting all of the functions, the definition of error semantics isn't brought up at all. For a Committed interface, I would expect that to be made explicit. I've asked some specific questions regarding return values in-line, but this is more of a general comment that I think needs to be addressed. > CLASSES > __builtin__.object > link > > class link(__builtin__.object) > | link(linkname, modes) -> link object > | > | Open linkname in specified mode. > | Three modes are supported: PASSIVE, RAW, NATIVE. > | See dlpi_open(3DLPI). in dlpi_open(3DLPI), the "modes" are actually "flags" that can be bitwise-OR'ed together to achieve a combination of behaviors (DLPI_PASSIVE, DLPI_RAW, and DLPI_NATIVE). I assume that this is also what's intended with "modes", although it isn't made explicit. On a related note, I'm no python expert, but aren't symbol names like "PASSIVE", "RAW", and "NATIVE" likely to clash with symbols defined by other classes potentially imported by an application? > | > | bind(...) > | bind(sap) -> unsigned int > | > | Attempts to bind the link to specified sap, or ANY_SAP. > | Returns the real sap gets bound. > | See dlpi_bind(3DLPI). Suggested rewording: "It returns the SAP that the function actually bound to, which could be different from the SAP requested." Also what happens on error? How does the application differentiate between errors and SAP values? > | > | disabmulti(...) > | disabmulti(address) -> None > | > | Disable a specified multicast address on this link. > | See dlpi_disabmulti(3DLPI). dlpi_disabmulti() can fail... > | > | enabmulti(...) > | enabmulti(address) -> None > | > | Enable a specified multicast address on this link. > | See dlpi_enabmulti(3DLPI). dlpi_enabmulti() can fail, and I think any sane application would want to know whether or not enabling multicast reception worked. Similar comment for other functions that don't have error semantics. > | > | enabnotify(...) > | enabnotify(events, function[, arg]) -> unsigned long > | > | Enables a notification callback for the set of specified > events, > | which must be one or more (by a logical OR) events listed > as below: > | NOTE_LINK_DOWN Notify when link has gone down > | NOTE_LINK_UP Notify when link has come up > | NOTE_PHYS_ADDR Notify when address changes > | NOTE_SDU_SIZE Notify when MTU changes > | NOTE_SPEED Notify when speed changes > | NOTE_PROMISC_ON_PHYS Notify when DL_PROMISC_PHYS is set > | NOTE_PROMISC_OFF_PHYS Notify when DL_PROMISC_PHYS is cleared > | Returns a handle for this registration. > | See dlpi_enabnotify(3DLPI). What happens if this fails? > | > | get_bcastaddr(...) > | get_bcastaddr() -> string > | > | Returns the broadcast address of the link. > | See dlpi_info(3DLPI). What is the link has no broadcast address? > | recv(...) > | recv(trunksize[, timeout]) -> (string, string) > | > | Attempts to receive message over this link. > | You need to specify the trunk size for the received message. > | And you can specify timeout value in seconds. > | Returns (source address, message data). > | See dlpi_recv(3DLPI). If timeout is not specified, does recv() behave as dlpi_recv() would if "msec" were 0 or -1 (i.e., does it not block, or does it block forever or until a message is received?). > | > | send(...) > | send(destaddr, message[, sap, minpri, maxpri]) -> None > | > | Attempts to send message over this link to destaddr. > | You can specify new sap, and priority range (minpri, maxpri). > | See dlpi_send(3DLPI). IMO, "you can specify new sap" is ambiguous. The idea is that a message is sent to the specified destination address and SAP. If the SAP isn't specified as the "sap" argument, then the bound SAP is used instead. At least that's what dlpi_send() does, and I think you want the same semantics. > DATA > ANY_SAP = 4294967295 > CURR_PHYS_ADDR = 2 > DEF_TIMEOUT = 5 > FACT_PHYS_ADDR = 1 > IDLE = 3 > NATIVE = 32 > NOTE_LINK_DOWN = 8 > NOTE_LINK_UP = 16 > NOTE_PHYS_ADDR = 1 > NOTE_PROMISC_OFF_PHYS = 4 > NOTE_PROMISC_ON_PHYS = 2 > NOTE_SDU_SIZE = 128 > NOTE_SPEED = 256 > PASSIVE = 2 > PROMISC_MULTI = 3 > PROMISC_PHYS = 1 > PROMISC_SAP = 2 > RAW = 4 > UNBOUND = 0 > UNKNOWN = -1 Are the values part of the committed interface? If not, then I would either remove the values from the case materials or explicitly state that only the symbol names are committed, and that their values are not an interface. -Seb
