Max Reitz <mre...@redhat.com> writes: > On 2018-05-10 16:54, Eric Blake wrote: >> On 05/09/2018 11:55 AM, Max Reitz wrote: >>> There are numerous QDict functions that have been introduced for and are >>> used only by the block layer. Move their declarations into an own >> >> s/an own/their own/ >> >>> header file to reflect that. >>> >>> While qdict_extract_subqdict() is in fact used outside of the block >>> layer (in util/qemu-config.c), it is still a function related very >>> closely to how the block layer works with nested QDicts, namely by >>> sometimes flattening them. Therefore, its declaration is put into this >>> header as well and util/qemu-config.c includes it with a comment stating >>> exactly which function it needs. >>> >>> Suggested-by: Markus Armbruster <arm...@redhat.com> >>> Signed-off-by: Max Reitz <mre...@redhat.com> >>> --- >> >>> +++ b/include/block/qdict.h >>> @@ -0,0 +1,35 @@ >>> +/* >>> + * Special QDict functions used by the block layer >>> + * >>> + * Copyright (c) 2018 Red Hat, Inc. >> >> You are extracting this from qdict.h which has: >> * Copyright (C) 2009 Red Hat Inc. >> >> Is it worth listing 2009-2018, instead of just this year? > > I don't know, is it? I don't know whether it makes a real difference.
Where's your taste for pedantry? Here's mine: please make it 2013-2018, because the oldest code moved is from 2013. >>> + * >>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or >>> later. >>> + * See the COPYING.LIB file in the top-level directory. >>> + */ >>> + >>> +#ifndef BLOCK_QDICT_H >>> +#define BLOCK_QDICT_H >>> + >>> +#include "qapi/qmp/qdict.h" >>> +#include "qapi/qmp/qobject.h" >>> +#include "qapi/error.h" Only the first #include is necessary, due to qemu/typedefs.h. Please drop the other two. >>> + >>> + >>> +void qdict_copy_default(QDict *dst, QDict *src, const char *key); >>> +void qdict_set_default_str(QDict *dst, const char *key, const char >>> *val); >> >> These two might be useful outside of the block layer; we can move them >> back in a later patch if so. But for now, I agree with your choice of >> moving them. >> >>> + >>> +void qdict_join(QDict *dest, QDict *src, bool overwrite); >> >> This is borderline whether it would be useful outside of the block layer. > > I decided I wanted to move the *_default* functions, and if I did that, > I would have to move this one as well. I decided I can't be biased just > because I wrote this one. :-) > > But in general I'd say if anyone wants to use any of these functions > outside of the block layer, they are welcome to move them back to the > main header, provided they have a good reason to do so. I suppose a > good reason for using qdict_join() may indeed turn up sooner or later. I like it this way. With the trivial changes I asked for: Reviewed-by: Markus Armbruster <arm...@redhat.com>