Messages in this thread | | | Date | Tue, 2 Nov 2021 22:02:10 +0200 | Subject | Re: [PATCH v2] nitro_enclaves: Fix implicit type conversion | From | "Paraschiv, Andra-Irina" <> |
| |
On 29/10/2021 04:48, Jiasheng Jiang wrote: > > The variable 'cpu' and 'cpu_sibling' are defined as unsigned int. > However in the for_each_cpu, their values are assigned to -1. > That doesn't make sense and in the cpumask_next() they are implicitly > type conversed to int. > It is universally accepted that the implicit type conversion is > terrible. > Also, having the good programming custom will set an example for > others. > Thus, it might be better to change the definition of 'cpu' and > 'cpu_sibling' from unsigned int to int. > > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn> > --- > drivers/virt/nitro_enclaves/ne_misc_dev.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c > index e21e1e8..2d80879 100644 > --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c > +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c > @@ -168,9 +168,9 @@ static bool ne_check_enclaves_created(void) > static int ne_setup_cpu_pool(const char *ne_cpu_list) > { > int core_id = -1; > - unsigned int cpu = 0; > + int cpu = 0; > cpumask_var_t cpu_pool; > - unsigned int cpu_sibling = 0; > + int cpu_sibling = 0; > unsigned int i = 0; > int numa_node = -1; > int rc = -EINVAL; > @@ -374,7 +374,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list) > */ > static void ne_teardown_cpu_pool(void) > { > - unsigned int cpu = 0; > + int cpu = 0; > unsigned int i = 0; > int rc = -EINVAL; > > @@ -516,7 +516,7 @@ static int ne_get_unused_core_from_cpu_pool(void) > static int ne_set_enclave_threads_per_core(struct ne_enclave *ne_enclave, > int core_id, u32 vcpu_id) > { > - unsigned int cpu = 0; > + int cpu = 0; > > if (core_id < 0 && vcpu_id == 0) { > dev_err_ratelimited(ne_misc_dev.this_device, > @@ -562,7 +562,7 @@ static int ne_set_enclave_threads_per_core(struct ne_enclave *ne_enclave, > static int ne_get_cpu_from_cpu_pool(struct ne_enclave *ne_enclave, u32 *vcpu_id) > { > int core_id = -1; > - unsigned int cpu = 0; > + int cpu = 0; > unsigned int i = 0; > int rc = -EINVAL; > > @@ -1017,7 +1017,7 @@ static int ne_start_enclave_ioctl(struct ne_enclave *ne_enclave, > struct ne_enclave_start_info *enclave_start_info) > { > struct ne_pci_dev_cmd_reply cmd_reply = {}; > - unsigned int cpu = 0; > + int cpu = 0; > struct enclave_start_req enclave_start_req = {}; > unsigned int i = 0; > struct pci_dev *pdev = ne_devs.ne_pci_dev->pdev; > @@ -1360,7 +1360,7 @@ static void ne_enclave_remove_all_mem_region_entries(struct ne_enclave *ne_encla > */ > static void ne_enclave_remove_all_vcpu_id_entries(struct ne_enclave *ne_enclave) > { > - unsigned int cpu = 0; > + int cpu = 0; > unsigned int i = 0; > > mutex_lock(&ne_cpu_pool.mutex); > -- > 2.7.4 >
Reviewed-by: Andra Paraschiv <andraprs@amazon.com>
As a follow-up, in general, when sending new versions of the patch(es) please also include the changelog after the "---" line. For example:
--- Changelog
v1 -> v2
* Change 1. * Change 2. ...........
That helps with patch(es) changes tracking.
Thanks, Andra
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
| |