Date: Fri, 05 Jul 2013 07:07:53 +0000 From: "Poul-Henning Kamp" <phk@phk.freebsd.dk> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: arch@FreeBSD.org Subject: Re: General purpose library for name/value pairs. Message-ID: <4818.1373008073@critter.freebsd.dk> In-Reply-To: <20130704215329.GG1402@garage.freebsd.pl> References: <20130704215329.GG1402@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <20130704215329.GG1402@garage.freebsd.pl>, Pawel Jakub Dawidek write s: >Currently we have plenty of random methods to pass data around: >- nmount(2) implements its own name/value pair approach, >- GEOM uses XML to pass data to userland, >- various sysctls and ioctls use binary structures to export data to > userland. You even overlooked a number of other marshalling implementations, NETGRAPH for instance. As should be obvious from the above list, where I'm guilty of most of it, I think this is a fundamentally a good and overdue idea. I am not entirely convinced that one-size-fits-all is possible, but a name/value list is basically what I ended up with both in nmount() and GEOM::gctl. Exporting complex state with XML is very hard to beat when it comes to code complexity in the kernel. For one thing, you'd need nested n/v lists which is where I decided in GEOM that XML was the way to go. Also, exporting performance counters via mmap(2) (see: devstat) is far superior to anything else, because it means no-syscall high-speed monitoring is possible. ifconfig(8) and netstat(1) could really use a total rewrite along those lines. So, yes I think your n/v idea has a lot of merits, but it's not going to be The Final Solution, and you should not force it on problems where we have better solutions. Implementation issues: Are duplicate names allowed, and if so, do they keep stable order ? Otherwise we will need some other API-convenient form of array-simulation, for instance for variable number of slices in GEOM slicers etc. >Note that we neither check if allocation succeeded nor individual >additions. The library is designed so that write-like operations can >gracefully handle previous failures. Yes, this is important, otherwise the error handling gets too maddening. Also remember a function for debugging which renders a nvlist_t (into a sbuf ?) >So, to add elements to the list one can use either nvlist_add_<type>() >or nvlist_move_<type>() functions. To get the values one also have two >choices: nvlist_get_<type>() and nvlist_take_<type>() - the former just >returns the value (but the value is still part of the nvlist) and the >latter removes associated nvpair from nvlist and returns its value. Do consider having these functions in a variant with a default argument, so that people don't have to wrap each optioanl n/v pair in an if(). One of the things you will need in the kernel is to check that sets of nv's contain what they should, before continuing processing. One particular thing to detect is extra, unwanted, elements, which at least represent a programmer mistake and which may become a security risk down the road, if they suddenly gets used. I would do this globally, by insisting that (kernel-)code cleans the nv lists it is passed, and then in syscall-return check that all nv-lists are in fact empty. Leaving it to programmers is more risky, most people will not grasp the need to be that careful for the long term, but in that case you will need functions like: int nvlist_accept_only(nvlist_t *, const char *, ...); which complains if there are any "strange" nv pairs. (see above for arrays, it should support that.) The complementary function, is also needed: int nvlist_has_all_of(nvlist_t *, const char *, ...); Finally, this kind of complex-data API causes errors which errno does not have the expressive power to communicate: "foo" not allowed when "bar" also specified Some years ago I proposed an API extension to allow the kernel/libs to return "detailed error strings" and I think this is a better idea than trying to shoe-horn errorstrings into individual API call. Something like: int error_buffer(char *, size_t len) Registers a char[] where kernel/libs can dump error strings. Thereafter, whenever a library or kernel call fail, you *MAY* have additional details in that bufffer: static char myerrbuf[1024]; [...] assert(0 == error_buffer(myerrbuf, sizeof myerrbuf)); strcpy(myerrbuf, "(No details available)"; [...] nvl = nvlist_recv(sock); if (nvl == NULL) err(1, "nvlist_recv() failed: %s", myerrbuf); [...] This is just to show the principle, it should be thought through, for instance maybe strerror() should know about and report the buffer contents, to get instant benefit without code changes ? And now for a really nasty question: Have you considered marshalling the n/v lists as JSON strings ? It would be pretty dang efficient just to stuff it into an sbuf and it would make transport into the kernel a breeze. Nobody says the kernel has to support anything but that exact JSON subset generated by your library functions, so parsing it it not too bad. Poul-Henning -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD committer | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4818.1373008073>