| Message ID | 20240608172142.138894-2-cgoettsche@seltendoof.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d034a3e66054 |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | [1/2] libsepol: move unchanged data out of loop | expand |
On Sat, Jun 8, 2024 at 1:22 PM Christian Göttsche <cgoettsche@seltendoof.de> wrote: > > From: Christian Göttsche <cgzones@googlemail.com> > > Check the class is defined once, and not for every permission via > is_perm_enabled(). Also pass the class datum to avoid an unnecessary > name lookup. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > .../include/sepol/policydb/avrule_block.h | 4 +-- > libsepol/src/avrule_block.c | 27 +++++++------------ > libsepol/src/link.c | 6 ++++- > 3 files changed, 17 insertions(+), 20 deletions(-) > > diff --git a/libsepol/include/sepol/policydb/avrule_block.h b/libsepol/include/sepol/policydb/avrule_block.h > index 27047d43..18a1dc78 100644 > --- a/libsepol/include/sepol/policydb/avrule_block.h > +++ b/libsepol/include/sepol/policydb/avrule_block.h > @@ -35,8 +35,8 @@ extern avrule_decl_t *get_avrule_decl(policydb_t * p, uint32_t decl_id); > extern cond_list_t *get_decl_cond_list(policydb_t * p, > avrule_decl_t * decl, > cond_list_t * cond); > -extern int is_id_enabled(char *id, policydb_t * p, int symbol_table); > -extern int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p); > +extern int is_id_enabled(const char *id, const policydb_t * p, int symbol_table); > +extern int is_perm_existent(const class_datum_t *cladatum, const char *perm_id); > > #ifdef __cplusplus > } > diff --git a/libsepol/src/avrule_block.c b/libsepol/src/avrule_block.c > index dcfce8b8..547021e9 100644 > --- a/libsepol/src/avrule_block.c > +++ b/libsepol/src/avrule_block.c > @@ -152,11 +152,11 @@ cond_list_t *get_decl_cond_list(policydb_t * p, avrule_decl_t * decl, > * marked as SCOPE_DECL, and any of its declaring block has been enabled, > * then return 1. Otherwise return 0. Can only be called after the > * decl_val_to_struct index has been created */ > -int is_id_enabled(char *id, policydb_t * p, int symbol_table) > +int is_id_enabled(const char *id, const policydb_t * p, int symbol_table) > { > - scope_datum_t *scope = > + const scope_datum_t *scope = > (scope_datum_t *) hashtab_search(p->scope[symbol_table].table, id); > - avrule_decl_t *decl; > + const avrule_decl_t *decl; > uint32_t len; > > if (scope == NULL) { > @@ -189,21 +189,14 @@ int is_id_enabled(char *id, policydb_t * p, int symbol_table) > return 0; > } > > -/* Check if a particular permission is present within the given class, > - * and that the class is enabled. Returns 1 if both conditions are > - * true, 0 if neither could be found or if the class id disabled. */ > -int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p) > +/* Check if a particular permission is present within the given class. > + * Whether the class is enabled is NOT checked. > + * Returns 1 if both conditions are true, > + * 0 if neither could be found or if the class id disabled. */ It returns 0 only if the permission is not found. Everything else looks good to me. Thanks, Jim > +int is_perm_existent(const class_datum_t *cladatum, const char *perm_id) > { > - class_datum_t *cladatum; > - perm_datum_t *perm; > - if (!is_id_enabled(class_id, p, SYM_CLASSES)) { > - return 0; > - } > - cladatum = > - (class_datum_t *) hashtab_search(p->p_classes.table, class_id); > - if (cladatum == NULL) { > - return 0; > - } > + const perm_datum_t *perm; > + > perm = hashtab_search(cladatum->permissions.table, perm_id); > if (perm == NULL && cladatum->comdatum != 0) { > /* permission was not in this class. before giving > diff --git a/libsepol/src/link.c b/libsepol/src/link.c > index a6f2a251..9281a986 100644 > --- a/libsepol/src/link.c > +++ b/libsepol/src/link.c > @@ -1968,6 +1968,10 @@ static int is_decl_requires_met(link_state_t * state, > id = pol->p_class_val_to_name[i]; > > > + if (!is_id_enabled(id, state->base, SYM_CLASSES)) { > + return 0; > + } > + > scope = hashtab_search(state->base->p_classes_scope.table, id); > if (scope == NULL) { > ERR(state->handle, > @@ -1994,7 +1998,7 @@ static int is_decl_requires_met(link_state_t * state, > perm_id = fparg.key; > > assert(perm_id != NULL); > - if (!is_perm_enabled(id, perm_id, state->base)) { > + if (!is_perm_existent(cladatum, perm_id)) { > if (req != NULL) { > req->symbol_type = SYM_CLASSES; > req->symbol_value = i + 1; > -- > 2.45.1 > >
On Tue, Jun 11, 2024 at 2:27 PM James Carter <jwcart2@gmail.com> wrote: > > On Sat, Jun 8, 2024 at 1:22 PM Christian Göttsche > <cgoettsche@seltendoof.de> wrote: > > > > From: Christian Göttsche <cgzones@googlemail.com> > > > > Check the class is defined once, and not for every permission via > > is_perm_enabled(). Also pass the class datum to avoid an unnecessary > > name lookup. > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> I will fix the comment when I merge the patch. Acked-by: James Carter <jwcart2@gmail.com> > > --- > > .../include/sepol/policydb/avrule_block.h | 4 +-- > > libsepol/src/avrule_block.c | 27 +++++++------------ > > libsepol/src/link.c | 6 ++++- > > 3 files changed, 17 insertions(+), 20 deletions(-) > > > > diff --git a/libsepol/include/sepol/policydb/avrule_block.h b/libsepol/include/sepol/policydb/avrule_block.h > > index 27047d43..18a1dc78 100644 > > --- a/libsepol/include/sepol/policydb/avrule_block.h > > +++ b/libsepol/include/sepol/policydb/avrule_block.h > > @@ -35,8 +35,8 @@ extern avrule_decl_t *get_avrule_decl(policydb_t * p, uint32_t decl_id); > > extern cond_list_t *get_decl_cond_list(policydb_t * p, > > avrule_decl_t * decl, > > cond_list_t * cond); > > -extern int is_id_enabled(char *id, policydb_t * p, int symbol_table); > > -extern int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p); > > +extern int is_id_enabled(const char *id, const policydb_t * p, int symbol_table); > > +extern int is_perm_existent(const class_datum_t *cladatum, const char *perm_id); > > > > #ifdef __cplusplus > > } > > diff --git a/libsepol/src/avrule_block.c b/libsepol/src/avrule_block.c > > index dcfce8b8..547021e9 100644 > > --- a/libsepol/src/avrule_block.c > > +++ b/libsepol/src/avrule_block.c > > @@ -152,11 +152,11 @@ cond_list_t *get_decl_cond_list(policydb_t * p, avrule_decl_t * decl, > > * marked as SCOPE_DECL, and any of its declaring block has been enabled, > > * then return 1. Otherwise return 0. Can only be called after the > > * decl_val_to_struct index has been created */ > > -int is_id_enabled(char *id, policydb_t * p, int symbol_table) > > +int is_id_enabled(const char *id, const policydb_t * p, int symbol_table) > > { > > - scope_datum_t *scope = > > + const scope_datum_t *scope = > > (scope_datum_t *) hashtab_search(p->scope[symbol_table].table, id); > > - avrule_decl_t *decl; > > + const avrule_decl_t *decl; > > uint32_t len; > > > > if (scope == NULL) { > > @@ -189,21 +189,14 @@ int is_id_enabled(char *id, policydb_t * p, int symbol_table) > > return 0; > > } > > > > -/* Check if a particular permission is present within the given class, > > - * and that the class is enabled. Returns 1 if both conditions are > > - * true, 0 if neither could be found or if the class id disabled. */ > > -int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p) > > +/* Check if a particular permission is present within the given class. > > + * Whether the class is enabled is NOT checked. > > + * Returns 1 if both conditions are true, > > + * 0 if neither could be found or if the class id disabled. */ > > It returns 0 only if the permission is not found. > > Everything else looks good to me. > Thanks, > Jim > > > > +int is_perm_existent(const class_datum_t *cladatum, const char *perm_id) > > { > > - class_datum_t *cladatum; > > - perm_datum_t *perm; > > - if (!is_id_enabled(class_id, p, SYM_CLASSES)) { > > - return 0; > > - } > > - cladatum = > > - (class_datum_t *) hashtab_search(p->p_classes.table, class_id); > > - if (cladatum == NULL) { > > - return 0; > > - } > > + const perm_datum_t *perm; > > + > > perm = hashtab_search(cladatum->permissions.table, perm_id); > > if (perm == NULL && cladatum->comdatum != 0) { > > /* permission was not in this class. before giving > > diff --git a/libsepol/src/link.c b/libsepol/src/link.c > > index a6f2a251..9281a986 100644 > > --- a/libsepol/src/link.c > > +++ b/libsepol/src/link.c > > @@ -1968,6 +1968,10 @@ static int is_decl_requires_met(link_state_t * state, > > id = pol->p_class_val_to_name[i]; > > > > > > + if (!is_id_enabled(id, state->base, SYM_CLASSES)) { > > + return 0; > > + } > > + > > scope = hashtab_search(state->base->p_classes_scope.table, id); > > if (scope == NULL) { > > ERR(state->handle, > > @@ -1994,7 +1998,7 @@ static int is_decl_requires_met(link_state_t * state, > > perm_id = fparg.key; > > > > assert(perm_id != NULL); > > - if (!is_perm_enabled(id, perm_id, state->base)) { > > + if (!is_perm_existent(cladatum, perm_id)) { > > if (req != NULL) { > > req->symbol_type = SYM_CLASSES; > > req->symbol_value = i + 1; > > -- > > 2.45.1 > > > >
diff --git a/libsepol/include/sepol/policydb/avrule_block.h b/libsepol/include/sepol/policydb/avrule_block.h index 27047d43..18a1dc78 100644 --- a/libsepol/include/sepol/policydb/avrule_block.h +++ b/libsepol/include/sepol/policydb/avrule_block.h @@ -35,8 +35,8 @@ extern avrule_decl_t *get_avrule_decl(policydb_t * p, uint32_t decl_id); extern cond_list_t *get_decl_cond_list(policydb_t * p, avrule_decl_t * decl, cond_list_t * cond); -extern int is_id_enabled(char *id, policydb_t * p, int symbol_table); -extern int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p); +extern int is_id_enabled(const char *id, const policydb_t * p, int symbol_table); +extern int is_perm_existent(const class_datum_t *cladatum, const char *perm_id); #ifdef __cplusplus } diff --git a/libsepol/src/avrule_block.c b/libsepol/src/avrule_block.c index dcfce8b8..547021e9 100644 --- a/libsepol/src/avrule_block.c +++ b/libsepol/src/avrule_block.c @@ -152,11 +152,11 @@ cond_list_t *get_decl_cond_list(policydb_t * p, avrule_decl_t * decl, * marked as SCOPE_DECL, and any of its declaring block has been enabled, * then return 1. Otherwise return 0. Can only be called after the * decl_val_to_struct index has been created */ -int is_id_enabled(char *id, policydb_t * p, int symbol_table) +int is_id_enabled(const char *id, const policydb_t * p, int symbol_table) { - scope_datum_t *scope = + const scope_datum_t *scope = (scope_datum_t *) hashtab_search(p->scope[symbol_table].table, id); - avrule_decl_t *decl; + const avrule_decl_t *decl; uint32_t len; if (scope == NULL) { @@ -189,21 +189,14 @@ int is_id_enabled(char *id, policydb_t * p, int symbol_table) return 0; } -/* Check if a particular permission is present within the given class, - * and that the class is enabled. Returns 1 if both conditions are - * true, 0 if neither could be found or if the class id disabled. */ -int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p) +/* Check if a particular permission is present within the given class. + * Whether the class is enabled is NOT checked. + * Returns 1 if both conditions are true, + * 0 if neither could be found or if the class id disabled. */ +int is_perm_existent(const class_datum_t *cladatum, const char *perm_id) { - class_datum_t *cladatum; - perm_datum_t *perm; - if (!is_id_enabled(class_id, p, SYM_CLASSES)) { - return 0; - } - cladatum = - (class_datum_t *) hashtab_search(p->p_classes.table, class_id); - if (cladatum == NULL) { - return 0; - } + const perm_datum_t *perm; + perm = hashtab_search(cladatum->permissions.table, perm_id); if (perm == NULL && cladatum->comdatum != 0) { /* permission was not in this class. before giving diff --git a/libsepol/src/link.c b/libsepol/src/link.c index a6f2a251..9281a986 100644 --- a/libsepol/src/link.c +++ b/libsepol/src/link.c @@ -1968,6 +1968,10 @@ static int is_decl_requires_met(link_state_t * state, id = pol->p_class_val_to_name[i]; + if (!is_id_enabled(id, state->base, SYM_CLASSES)) { + return 0; + } + scope = hashtab_search(state->base->p_classes_scope.table, id); if (scope == NULL) { ERR(state->handle, @@ -1994,7 +1998,7 @@ static int is_decl_requires_met(link_state_t * state, perm_id = fparg.key; assert(perm_id != NULL); - if (!is_perm_enabled(id, perm_id, state->base)) { + if (!is_perm_existent(cladatum, perm_id)) { if (req != NULL) { req->symbol_type = SYM_CLASSES; req->symbol_value = i + 1;