Hello, Am Mittwoch, 3. Juni 2015 schrieb Steve Beattie: > On Mon, May 25, 2015 at 01:36:00PM +0200, Christian Boltz wrote: > > this patch adds two variable references (aa and changed) in > > aa-mergeprof ask_the_questions() so that the code can use the short > > name and be more in sync with aa.py ask_the_questions(). > > > > With this patch applied, the "for ruletype in ['capability', > > 'network']:" block is in sync, with the exception of the sections > > that intentionally differ: > > - the check for the profile mode > > - the default button selection based on profile mode > > - the seen_events counter > > > > [ 30-mergeprof-sync-varnames.diff ] > > I get why you're doing it, but it's a little goofy to convert only > some of the references to self.user.aa and apparmor.aa.changed in > aa-mergeprof::ask_the_questions() and not all of them. I'd personally > rather see a larger patch that converts all the references in > ask_the_questions().
As discussed on IRC, my plan was only to change sections that I touched and tested in this patch series. Wearing my "you wanted it so" hat, here's the updated patch that changes the variable names in the whole ask_the_questions() function: Get variable names in aa-mergeprof ask_the_questions() in sync with aa.py Add two variable references (aa and changed) in aa-mergeprof ask_the_questions() so that the code can use the short name and be more in sync with aa.py ask_the_questions(). With this patch applied, the "for ruletype in ['capability', 'network']:" block is in sync, with the exception of the sections that intentionally differ: - the check for the profile mode - the default button selection based on profile mode - the seen_events counter The patch also includes some minor whitespace fixes. [ 30-mergeprof-sync-varnames.diff ] === modified file utils/aa-mergeprof --- utils/aa-mergeprof 2015-06-05 23:23:08.208668057 +0200 +++ utils/aa-mergeprof 2015-06-05 23:35:55.953845107 +0200 @@ -235,6 +235,9 @@ done = True def ask_the_questions(self, other, profile): + aa = self.user.aa # keep references so that the code in this function can use the short name + changed = apparmor.aa.changed # (and be more in sync with aa.py ask_the_questions()) + if other == 'other': other = self.other else: @@ -281,7 +284,7 @@ options = [] for inc in other.aa[profile][hat]['include'].keys(): - if not inc in self.user.aa[profile][hat]['include'].keys(): + if not inc in aa[profile][hat]['include'].keys(): options.append('#include <%s>' %inc) default_option = 1 @@ -300,8 +303,8 @@ elif ans == 'CMD_ALLOW': selection = options[selected] inc = apparmor.aa.re_match_include(selection) - deleted = apparmor.aa.delete_duplicates(self.user.aa[profile][hat], inc) - self.user.aa[profile][hat]['include'][inc] = True + deleted = apparmor.aa.delete_duplicates(aa[profile][hat], inc) + aa[profile][hat]['include'][inc] = True options.pop(selected) aaui.UI_Info(_('Adding %s to the file.') % selection) if deleted: @@ -315,10 +318,10 @@ #print(path, other.aa[profile][hat][allow]['path'][path]) mode = other.aa[profile][hat][allow]['path'][path]['mode'] - if self.user.aa[profile][hat][allow]['path'].get(path, False): - mode = self.conflict_mode(profile, hat, allow, path, 'mode', other.aa[profile][hat][allow]['path'][path]['mode'], self.user.aa[profile][hat][allow]['path'][path]['mode']) - self.conflict_mode(profile, hat, allow, path, 'audit', other.aa[profile][hat][allow]['path'][path]['audit'], self.user.aa[profile][hat][allow]['path'][path]['audit']) - apparmor.aa.changed[profile] = True + if aa[profile][hat][allow]['path'].get(path, False): + mode = self.conflict_mode(profile, hat, allow, path, 'mode', other.aa[profile][hat][allow]['path'][path]['mode'], aa[profile][hat][allow]['path'][path]['mode']) + self.conflict_mode(profile, hat, allow, path, 'audit', other.aa[profile][hat][allow]['path'][path]['audit'], aa[profile][hat][allow]['path'][path]['audit']) + changed[profile] = True continue # Lookup modes from profile allow_mode = set() @@ -326,25 +329,25 @@ deny_mode = set() deny_audit = set() - fmode, famode, fm = apparmor.aa.rematchfrag(self.user.aa[profile][hat], 'allow', path) + fmode, famode, fm = apparmor.aa.rematchfrag(aa[profile][hat], 'allow', path) if fmode: allow_mode |= fmode if famode: allow_audit |= famode - cm, cam, m = apparmor.aa.rematchfrag(self.user.aa[profile][hat], 'deny', path) + cm, cam, m = apparmor.aa.rematchfrag(aa[profile][hat], 'deny', path) if cm: deny_mode |= cm if cam: deny_audit |= cam - imode, iamode, im = apparmor.aa.match_prof_incs_to_path(self.user.aa[profile][hat], 'allow', path) + imode, iamode, im = apparmor.aa.match_prof_incs_to_path(aa[profile][hat], 'allow', path) if imode: allow_mode |= imode if iamode: allow_audit |= iamode - cm, cam, m = apparmor.aa.match_prof_incs_to_path(self.user.aa[profile][hat], 'deny', path) + cm, cam, m = apparmor.aa.match_prof_incs_to_path(aa[profile][hat], 'deny', path) if cm: deny_mode |= cm if cam: @@ -385,7 +388,7 @@ for incname in apparmor.aa.include.keys(): include_valid = False # If already present skip - if self.user.aa[profile][hat][incname]: + if aa[profile][hat][incname]: continue if incname.startswith(apparmor.aa.profile_dir): incname = incname.replace(apparmor.aa.profile_dir+'/', '', 1) @@ -435,7 +438,7 @@ owner_toggle = apparmor.aa.cfg['settings']['default_owner_prompt'] done = False while not done: - q = aaui.PromptQuestion() + q = aaui.PromptQuestion() q.headers = [_('Profile'), apparmor.aa.combine_name(profile, hat), _('Path'), path] @@ -524,26 +527,26 @@ match = apparmor.aa.re_match_include(path) if match: inc = match - deleted = apparmor.aa.delete_duplicates(self.user.aa[profile][hat], inc) - self.user.aa[profile][hat]['include'][inc] = True - apparmor.aa.changed[profile] = True + deleted = apparmor.aa.delete_duplicates(aa[profile][hat], inc) + aa[profile][hat]['include'][inc] = True + changed[profile] = True aaui.UI_Info(_('Adding %s to profile.') % path) if deleted: aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted) else: - if self.user.aa[profile][hat]['allow']['path'][path].get('mode', False): - mode |= self.user.aa[profile][hat]['allow']['path'][path]['mode'] + if aa[profile][hat]['allow']['path'][path].get('mode', False): + mode |= aa[profile][hat]['allow']['path'][path]['mode'] deleted = [] - for entry in self.user.aa[profile][hat]['allow']['path'].keys(): + for entry in aa[profile][hat]['allow']['path'].keys(): if path == entry: continue if apparmor.aa.matchregexp(path, entry): - if apparmor.aa.mode_contains(mode, self.user.aa[profile][hat]['allow']['path'][entry]['mode']): + if apparmor.aa.mode_contains(mode, aa[profile][hat]['allow']['path'][entry]['mode']): deleted.append(entry) for entry in deleted: - self.user.aa[profile][hat]['allow']['path'].pop(entry) + aa[profile][hat]['allow']['path'].pop(entry) deleted = len(deleted) if owner_toggle == 0: @@ -555,19 +558,19 @@ elif owner_toggle == 3: mode = apparmor.aa.owner_flatten_mode(mode) - if not self.user.aa[profile][hat]['allow'].get(path, False): - self.user.aa[profile][hat]['allow']['path'][path]['mode'] = self.user.aa[profile][hat]['allow']['path'][path].get('mode', set()) | mode + if not aa[profile][hat]['allow'].get(path, False): + aa[profile][hat]['allow']['path'][path]['mode'] = aa[profile][hat]['allow']['path'][path].get('mode', set()) | mode tmpmode = set() if audit_toggle == 1: - tmpmode = mode- allow_mode + tmpmode = mode - allow_mode elif audit_toggle == 2: tmpmode = mode - self.user.aa[profile][hat]['allow']['path'][path]['audit'] = self.user.aa[profile][hat]['allow']['path'][path].get('audit', set()) | tmpmode + aa[profile][hat]['allow']['path'][path]['audit'] = aa[profile][hat]['allow']['path'][path].get('audit', set()) | tmpmode - apparmor.aa.changed[profile] = True + changed[profile] = True aaui.UI_Info(_('Adding %(path)s %(mode)s to profile') % { 'path': path, 'mode': apparmor.aa.mode_to_str_user(mode) }) if deleted: @@ -576,11 +579,11 @@ elif ans == 'CMD_DENY': path = options[selected].strip() # Add new entry? - self.user.aa[profile][hat]['deny']['path'][path]['mode'] = self.user.aa[profile][hat]['deny']['path'][path].get('mode', set()) | (mode - allow_mode) + aa[profile][hat]['deny']['path'][path]['mode'] = aa[profile][hat]['deny']['path'][path].get('mode', set()) | (mode - allow_mode) - self.user.aa[profile][hat]['deny']['path'][path]['audit'] = self.user.aa[profile][hat]['deny']['path'][path].get('audit', set()) + aa[profile][hat]['deny']['path'][path]['audit'] = aa[profile][hat]['deny']['path'][path].get('audit', set()) - apparmor.aa.changed[profile] = True + changed[profile] = True done = True @@ -627,12 +630,12 @@ if other.aa[profile][hat].get(ruletype, False): # needed until we have proper profile initialization for rule_obj in other.aa[profile][hat][ruletype].rules: - if is_known_rule(self.user.aa[profile][hat], ruletype, rule_obj): + if is_known_rule(aa[profile][hat], ruletype, rule_obj): continue default_option = 1 options = [] - newincludes = match_includes(self.user.aa[profile][hat], ruletype, rule_obj) + newincludes = match_includes(aa[profile][hat], ruletype, rule_obj) q = aaui.PromptQuestion() if newincludes: options += list(map(lambda inc: '#include <%s>' % inc, sorted(set(newincludes)))) @@ -675,32 +678,32 @@ elif ans == 'CMD_ALLOW': done = True - apparmor.aa.changed[profile] = True + changed[profile] = True selection = options[selected] inc = re_match_include(selection) if inc: - deleted = delete_duplicates(self.user.aa[profile][hat], inc) + deleted = delete_duplicates(aa[profile][hat], inc) - self.user.aa[profile][hat]['include'][inc] = True + aa[profile][hat]['include'][inc] = True aaui.UI_Info(_('Adding %s to profile.') % selection) if deleted: aaui.UI_Info(_('Deleted %s previous matching profile entries.') % deleted) else: - self.user.aa[profile][hat][ruletype].add(rule_obj) + aa[profile][hat][ruletype].add(rule_obj) aaui.UI_Info(_('Adding %s to profile.') % rule_obj.get_clean()) elif ans == 'CMD_DENY': done = True - apparmor.aa.changed[profile] = True + changed[profile] = True rule_obj.deny = True rule_obj.raw_rule = None # reset raw rule after manually modifying rule_obj - self.user.aa[profile][hat][ruletype].add(rule_obj) + aa[profile][hat][ruletype].add(rule_obj) aaui.UI_Info(_('Adding %s to profile.') % rule_obj.get_clean()) else: Regards, Christian Boltz -- As usual, I'm voicing my opinion on this topic, not announcing "truth" ;) [Cristian RodrÃguez in opensuse-factory] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor