[apparmor] [patch] better error messages in aa.py store_list_var()

[Steve Beattie]

On Thu, Jun 19, 2014 at 09:33:59PM +0200, Christian Boltz wrote:
> Hello,
> 
> Am Mittwoch, 18. Juni 2014 schrieb Steve Beattie:
> > On Thu, Jun 19, 2014 at 02:41:39AM +0200, Christian Boltz wrote:
> > > this patch improves the error messages in aa.py store_list_var() to
> > > make debugging of profile syntax problems easier.
> > 
> > This is an okay improvement as-is, but it sure would be nice if we
> > could the name of the problematic file being parsed attached to the
> > exception.  Maybe in a try-except block, the callers could re-raise
> > the exception with the filename attached?
> 
> Good idea, except that I prefer an additional parameter over the
> try/except game in this case.

Sure, that's a reasonable approach, too.

> Updated patch:

Acked-by: Steve Beattie <steve at nxnw.org>

Thanks!

> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py        2014-06-19 18:44:57 +0000
> +++ utils/apparmor/aa.py        2014-06-19 19:26:18 +0000
> @@ -2851,11 +2851,11 @@
>              if profile:
>                  if not profile_data[profile][hat].get('lvar', False):
>                      profile_data[profile][hat]['lvar'][list_var] = []
> -                store_list_var(profile_data[profile]['lvar'], list_var, value, var_operation)
> +                store_list_var(profile_data[profile]['lvar'], list_var, value, var_operation, file)
>              else:
>                  if not filelist[file].get('lvar', False):
>                      filelist[file]['lvar'][list_var] = []
> -                store_list_var(filelist[file]['lvar'], list_var, value, var_operation)
> +                store_list_var(filelist[file]['lvar'], list_var, value, var_operation, file)
>  
>          elif RE_PROFILE_CONDITIONAL.search(line):
>              # Conditional Boolean
> @@ -3245,7 +3245,7 @@
>      else:
>          return False
>  
> -def store_list_var(var, list_var, value, var_operation):
> +def store_list_var(var, list_var, value, var_operation, filename):
>      """Store(add new variable or add values to variable) the variables encountered in the given list_var"""
>      vlist = separate_vars(value)
>      if var_operation == '=':
> @@ -3253,14 +3253,14 @@
>              var[list_var] = set(vlist)
>          else:
>              #print('Ignored: New definition for variable for:',list_var,'=', value, 'operation was:',var_operation,'old value=', var[list_var])
> -            raise AppArmorException(_('An existing variable redefined: %s') % list_var)
> +            raise AppArmorException(_('Redefining existing variable %s: %s in %s') % (list_var, value, filename))
>      elif var_operation == '+=':
>          if var.get(list_var, False):
>              var[list_var] = set(var[list_var] + vlist)
>          else:
> -            raise AppArmorException(_('Values added to a non-existing variable: %s') % list_var)
> +            raise AppArmorException(_('Values added to a non-existing variable %s: %s in %s') % (list_var, value, filename))
>      else:
> -        raise AppArmorException(_('Unknown variable operation: %s') % var_operation)
> +        raise AppArmorException(_('Unknown variable operation %s for variable %s in %s') % (var_operation, list_var, filename))
>  
>  
>  def strip_quotes(data):
> @@ -4097,11 +4097,11 @@
>                  value = strip_quotes(matches[2])
>                  var_set = hasher()
>                  if profile:
> -                    store_list_var(var_set, list_var, value, var_operation)
> +                    store_list_var(var_set, list_var, value, var_operation, prof_filename)
>                      if not var_set[list_var] == write_prof_data['lvar'].get(list_var, False):
>                          correct = False
>                  else:
> -                    store_list_var(var_set, list_var, value, var_operation)
> +                    store_list_var(var_set, list_var, value, var_operation, prof_filename)
>                      if not var_set[list_var] == write_filelist['lvar'].get(list_var, False):
>                          correct = False
>  
> 
> Regards,
> 
> Christian Boltz
> -- 
> Spätestens dabei handelt es sich um Filtereffekte, die ImageMagick
> bestimmt nicht beherrschen kann. Sollten sie _das_ nachprogrammiert
> haben, würde ich barfuß hinlaufen und ihnen ein halbes Schwein
> opfern ob ihrer Genialität. [Ratti in suse-linux]
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140619/dcbed2bf/attachment.pgp>