[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [patch] better error messages in aa.py store_list_var()
From:       Steve Beattie <steve () nxnw ! org>
Date:       2014-06-19 22:54:01
Message-ID: 20140619225401.GA3900 () nxnw ! org
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


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@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@lists.ubuntu.com
> Modify settings or unsubscribe at: \
> https://lists.ubuntu.com/mailman/listinfo/apparmor

-- 
Steve Beattie
<sbeattie@ubuntu.com>
http://NxNW.org/~steve/


["signature.asc" (application/pgp-signature)]

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor


[prev in list] [next in list] [prev in thread] [next in thread]