[PATCH 1/3] libshell: fix subtle section parsing bug

[Keller, Jacob E]

> -----Original Message-----
> From: Alexey Gladkov [mailto:legion at altlinux.ru]
> Sent: Wednesday, May 28, 2014 6:10 AM
> To: dedekind1 at gmail.com; Keller, Jacob E
> Cc: aiaiai at lists.infradead.org
> Subject: Re: [PATCH 1/3] libshell: fix subtle section parsing bug
> 
> 28.05.2014 11:10, кон Bityutskiy wrote:
> > Hi Jacob,
> >
> > I never had time for this, but wanted to let you know the following.
> >
> > 1. We carry a copy of rather old libshell. The upstream repo of it is
> > this:
> >
> > git://git.altlinux.org/people/legion/packages/libshell.git
> >
> > We may want to look there and update sometimes.
> 
> also I have a mirror at github:
> 
> https://github.com/legionus/libshell
> 
> > 2. The libshell contact is: Alexey Gladkov ⟨legion at altlinux.org⟩
> > May be it makes sense to CC Alexey in cases like this. Let me do just
> > that.
> >
> > Thanks!
> >
> >
> > On Tue, 2014-05-27 at 15:17 -0700, Jacob Keller wrote:
> >> The libshell ini_config_get and ini_config_is_set functions were
> broken
> >> in a subtle way, due to allowance of variables in a section which may
> >> not be defined. When we encountered the [section] tag that matched
> our
> >> given section, we enabled reading each line to check the variables.
> >> However, we never disabled the section once we found it, so it was
> >> possible for a variable defined in a forthcoming section to be
> returned,
> >> if we didn't define the variable in the expected section.
> >>
> >> This resulted in several subtle bugs regarding project configuration
> >> especially regarding the [default] section, and multiple projects. The
> >> fix for this, is simply to disable section processing upon section
> >> change. I thought about possibly just exiting, if we've found our
> >> section and it changes. However, that is a bit more complex logic, and
> >> this fix allows for having the same section twice, which might be a
> >> little confusing, but I think it's acceptable.
> >>
> >> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> >> ---
> >>  helpers/libshell/shell-ini-config | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/helpers/libshell/shell-ini-config b/helpers/libshell/shell-ini-
> config
> >> index ba0bfbde777a..ffce51598876 100644
> >> --- a/helpers/libshell/shell-ini-config
> >> +++ b/helpers/libshell/shell-ini-config
> >> @@ -21,6 +21,8 @@ ini_config_get() {
> >>
> >>  		case "$str" in
> >>  			"["*"]")
> >> +				# Reset section processing on section
> change
> >> +				sect=
> >>  				[ "$str" != "[$section]" ] ||
> >>  					sect=1
> >>  				;;
> 
> Yes. It makes sense.
> 
> >> @@ -52,6 +54,8 @@ ini_config_is_set()
> >>
> >>  		case "$str" in
> >>  			"["*"]")
> >> +				# Reset section processing on section
> change
> >> +				sect=
> >>  				[ "$str" != "[$section]" ] ||
> >>  					sect=1
> >>  				;;
> 
> This code is very old. ini_config_is_set has no more of this code.
> 

Actually, ini_config_is_set is something I added to the code as our use case required being able to distinguish between a set but empty field or a non-set field.

> You can make a patch against recent version ?
> 
> I don't know your usecase, but if you talk about the subsections, then
> you may be interested in shell-git-config (parser of git-like config
> files).
> 

That might be better for our case, as the git-like configuration is a bit more powerful.

Thanks,
Jake

> --
> Rgrds, legion
> 
> 
> --
> Rgrds, legion