Date: Tue, 09 Dec 2014 21:29:28 +0100 From: Stefan Esser <se@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org>, Phil Shafer <phil@juniper.net> Subject: Re: [Patch] updated: Add JSON and XML output to pciconf (libxo support - D1206) Message-ID: <54875BA8.1040600@freebsd.org> In-Reply-To: <20141209164516.GB97072@kib.kiev.ua> References: <201412082114.sB8LEJnR056654@idle.juniper.net> <548723D0.1000402@freebsd.org> <20141209164516.GB97072@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Am 09.12.2014 um 17:45 schrieb Konstantin Belousov: > On Tue, Dec 09, 2014 at 05:31:12PM +0100, Stefan Esser wrote: >> Am 08.12.2014 um 22:14 schrieb Phil Shafer: >>> Stefan Esser writes: >>>> D1206 on reviews.freebsd.org (https://reviews.freebsd.org/D1206). >>> >>> Patch looks good. Some nits: >> >> Hi Phil, >> >> thanks again for your effort to help me understand libxo! >> >>> The names for xo_open_list and xo_open_instance need to be >>> identical, so: >>> >>>> @@ -554,9 +698,14 @@ >>>> /* Walk the capability list. */ >>>> express = 0; >>>> ptr = read_config(fd, &p->pc_sel, ptr, 1); >>>> + xo_open_list("capabilities"); >>> >>> should be: >>> >>> xo_open_list("capability"); >>> >>> to match: >>> >>>> + xo_open_instance("capability"); >> >> I just re-read the man-page for xo_list_open() and it clearly says: >> >> "The name given to all calls must be identical, and it is strong >> suggested that the name be singular, not plural, [...]", which I had >> wrongly understood to only relate to opening and closing of elements. > If it is required that the name be identical, it makes more sense > to require some handle to be passed to functions instead of the string. > I.e. either there should be some atom-like calls which convert string > to identifier, or first call should return handle passed to consequent > calls. > > This is obviously a note WRT libxo interface, not your patch. Use of handles could help with other aspects of the conversion, too. (E.g. to build several trees of information in parallel and then flush them to output as several lists in some specified order - I have, there is one loop in pciconf that prints 3 kinds of data as text that need to be separated for structured output, for example). Such sub-tree handles could also simplify the task of converting for libxo (modelled after DOM based XML generators, instead of the current SAX like style). The "handles" that exist in libxo serve a different purpose and allow e.g. several independent output streams to be written to by a single program. But the current interface "as is" is very regular with regard to the parameters passed and the xolint tool really helps to identify bad format strings and other mistakes that are hard to spot, otherwise. [...] >> OTOH, I'd love to see the output generated on an AMD64 system >> with HyperThreading information. > There are no AMD processors which support HyperThreading. I suspect you > mean HyperTransport there. Any relatively modern AMD CPU, in particular, > all chips capable of running amd64, use hypertransport for northbridge. Yes, sure, I meant HyperTransport (and spelled it that way in the patch). I do not have any AMD systems to test on (but I have an old nForce 630a/7050 mainboard and all parts to reconstruct a working AMD64 X2 (AM2) system, ... but not this week). > Look at netperf cluster for available resources, there are several > AMD machines there > https://wiki.freebsd.org/TestClusterOneReservations Thank you for the pointer - I'll ask for test access (I'll need root access, albeit only for a few minutes) to a suitable machine in the cluster, next week. Best regards, STefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54875BA8.1040600>