On (02/11/09 16:07), Peter Memishian wrote: > > > > > http://cr.opensolaris.org/~sowmini/parsegoop > > > > http://zhadum.east/export/ws/sowmini/brussels/parsegoop/webrev
In addtion to updating the above webrevs, I have the incremental webrev available internally at http://zhadum.east.sun.com/export/ws/sowmini/brussels/parsegoop2cv.next/web.delta/ As you pointed out, I no longer needed the prototype definitions in ipmpstat.c. I also fixed the nits. > Regarding the ofmt API, for ofmt_open(): > > Would recommend that the `parsable' boolean_t be generalized > into a flags field so that it's clear what "B_TRUE" means and > easier to extend in the future. (In general, we've found a > flags argument every useful in APIs like dlpi_open()). > > Would recommend that the column width be an argument. The > caller can request a specific width or OFMT_DYNWIDTH or somesuch > to ask ofmt to use the width provided by ofmt_update_winsize() > calls. > I added a new flags argument that takes OFMT_PARSABLE and OFMT_DYNWIDTH. But I think that allowing both the OFMT_DYNWIDTH and a fields arg is allowing too many confusing degrees of freedom esp if you also throw in a non-null fields_str that strictly specifies what fields to output. I think the following model is simpler and more useful - if fields_str is non-null, output columns selected; it's the caller's job to make sure the output fits within the window. - if fields_str is NULL, and OFMT_DYNWIDTH is *not* specified, output all columns. Again, its the caller's job to stretch the window if needed. - if fields_str is NULL, and OFMT_DYNWIDTH is specified, adjust to window size. This allows both the ipmpstat behavior (where it fits to window size) and the dladm/flowadm behavior (where it expects the user to stretch the window as needed) --Sowmini
