Re: [PATCH v4 2/5] cfgfile: change existing API functions
From: Bruce Richardson <hidden>
Date: 2017-08-30 20:07:32
I think the commit title needs rewording. This changes the internals not the API. On Mon, Jul 10, 2017 at 02:44:14PM +0200, Jacek Piasecki wrote:
Change to flat arrays in cfgfile struct force slightly diffrent data access for most of cfgfile functions. This patch provides necessary changes in existing API. Signed-off-by: Jacek Piasecki <redacted> ---
Some comments below. I believe the change in return value from -1 to -EINVAL, though a more correct error, actually counts as an ABI change, so I think that should be removed, i.e. keep errors at -1. Once done: Acked-by: Bruce Richardon <redacted>
quoted hunk ↗ jump to hunk
lib/librte_cfgfile/rte_cfgfile.c | 120 +++++++++++++++++++-------------------- lib/librte_cfgfile/rte_cfgfile.h | 6 +- 2 files changed, 62 insertions(+), 64 deletions(-)diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c index c6ae3e3..50fe37a 100644 --- a/lib/librte_cfgfile/rte_cfgfile.c +++ b/lib/librte_cfgfile/rte_cfgfile.c@@ -35,6 +35,7 @@ #include <stdlib.h> #include <string.h> #include <ctype.h> +#include <errno.h> #include <rte_common.h> #include "rte_cfgfile.h"@@ -42,13 +43,15 @@ struct rte_cfgfile_section { char name[CFG_NAME_LEN]; int num_entries; - struct rte_cfgfile_entry *entries[0]; + int allocated_entries; + struct rte_cfgfile_entry *entries; }; struct rte_cfgfile { int flags; int num_sections; - struct rte_cfgfile_section *sections[0]; + int allocated_sections; + struct rte_cfgfile_section *sections; };
These are good changes, allowing us to have the sections array and entries arrays separate from the basic data structures. <snip>
quoted hunk ↗ jump to hunk
@@ -409,7 +407,7 @@ rte_cfgfile_section_num_entries(struct rte_cfgfile *cfg, { const struct rte_cfgfile_section *s = _get_section(cfg, sectionname); if (s == NULL) - return -1; + return -EINVAL; return s->num_entries; }
I think this change should be dropped for backward compatibility, or else put in NEXT_ABI #ifdefs and an ABI notice added to the RN.