On Thu, 2016-02-11 at 01:55 +0200, Dmitry Kasatkin wrote: > On Feb 11, 2016 1:22 AM, "Mimi Zohar" <zohar at linux.vnet.ibm.com> wrote: > > > > On Wed, 2016-02-10 at 23:09 +0200, Dmitry Kasatkin wrote: > > > On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar <zohar at linux.vnet.ibm.com> > wrote: > > > > > > > > - if (read_id == READING_FIRMWARE) > > > > + switch (read_id) { > > > > + case READING_FIRMWARE: > > > > func = FIRMWARE_CHECK; > > > > - else if (read_id == READING_MODULE) > > > > + break; > > > > + case READING_MODULE: > > > > func = MODULE_CHECK; > > > > + break; > > > > + case READING_KEXEC_IMAGE: > > > > + func = KEXEC_CHECK; > > > > + break; > > > > + case READING_KEXEC_INITRAMFS: > > > > + func = INITRAMFS_CHECK; > > > > + break; > > > > + default: > > > > + func = FILE_CHECK; > > > > + break; > > > > + } > > > > > > > > > > I would define a separate function like "int ima_read_id_to_func(id)" > > > which search over the map > > > > > > Something like... > > > > > > struct > > > { > > > int id; > > > int func; > > > } map[] = { > > > { .id = READING_FIRMWARE, .fun = FIRMWARE_CHECK }, > > > ... > > > { -1, 0 } > > > }; > > > > > > > So we stay with the duplication (option 1), but clean it up. That works > > for me. > > > > Actually it may be simpler. > Just define int idmap[MAX_ID] and assign to every id corresponding func. > It will be quick and simple. Unlike the ima_read_id_to_func() above or the original switch/case statement, this method assumes the kernel_read_file_id enumeration stays in sync with ima_hooks. In terms of the ima_read_id_to_func() function, it would iterate over the map[] to find the corresponding .id, whereas the current switch/case is a direct lookup. Perhaps we should defer making a change for now. Mimi