[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH] xen/arm: address violation of MISRA C Rule 10.1
On 11.07.2025 02:15, Stefano Stabellini wrote: > On Thu, 10 Jul 2025, Dmytro Prokopchuk1 wrote: >> --- a/xen/common/memory.c >> +++ b/xen/common/memory.c >> @@ -773,7 +773,7 @@ static long >> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg) >> >> nrspin_lock(&d->page_alloc_lock); >> drop_dom_ref = (dec_count && >> - !domain_adjust_tot_pages(d, -dec_count)); >> + !domain_adjust_tot_pages(d, (-1L) * >> dec_count)); > > ...but I don't understand this? It looks like it would also break 10.1 > and/or 10.3 as well? > > I would rather use casts if needed, which wouldn't change the behavior > but would highlight the unsigned->signed conversion, making it more > visible: > > !domain_adjust_tot_pages(d, -(int)dec_count)); Except that, as more often than not anyway, such casts are fragile. >> --- a/xen/common/page_alloc.c >> +++ b/xen/common/page_alloc.c >> @@ -691,7 +691,7 @@ size_param("low_mem_virq_limit", opt_low_mem_virq); >> /* Thresholds to control hysteresis. In pages */ >> /* When memory grows above this threshold, reset hysteresis. >> * -1 initially to not reset until at least one virq issued. */ >> -static unsigned long low_mem_virq_high = -1UL; >> +static unsigned long low_mem_virq_high = ~0UL; >> /* Threshold at which we issue virq */ >> static unsigned long low_mem_virq_th = 0; >> /* Original threshold after all checks completed */ >> @@ -710,7 +710,7 @@ static void __init setup_low_mem_virq(void) >> * to ever trigger. */ >> if ( opt_low_mem_virq == 0 ) >> { >> - low_mem_virq_th = -1UL; >> + low_mem_virq_th = ~0UL; >> return; >> } >> >> @@ -778,7 +778,7 @@ static void check_low_mem_virq(void) >> low_mem_virq_th_order++; >> low_mem_virq_th = 1UL << low_mem_virq_th_order; >> if ( low_mem_virq_th == low_mem_virq_orig ) >> - low_mem_virq_high = -1UL; >> + low_mem_virq_high = ~0UL; >> else >> low_mem_virq_high = 1UL << (low_mem_virq_th_order + 2); >> } >> --- a/xen/common/time.c >> +++ b/xen/common/time.c >> @@ -84,7 +84,7 @@ struct tm gmtime(unsigned long t) >> } >> tbuf.tm_year = y - 1900; >> tbuf.tm_yday = days; >> - ip = (const unsigned short int *)__mon_lengths[__isleap(y)]; >> + ip = (const unsigned short int *)__mon_lengths[(int)__isleap(y)]; > > __isleap return bool and we deviated bool conversions in logical > operations but not here, so I understand why this is needed. OK. No. If a cast is unavoidable here (which I question), it wants to be to an unsigned type. I'm glad I looked at the patch, btw, since these files - contrary to the patch subject prefix - aren't Arm files. >> --- a/xen/lib/strtol.c >> +++ b/xen/lib/strtol.c >> @@ -13,7 +13,7 @@ >> long simple_strtol(const char *cp, const char **endp, unsigned int base) >> { >> if ( *cp == '-' ) >> - return -simple_strtoul(cp + 1, endp, base); >> + return (-1L) * simple_strtoul(cp + 1, endp, base); >> return simple_strtoul(cp, endp, base); >> } >> >> --- a/xen/lib/strtoll.c >> +++ b/xen/lib/strtoll.c >> @@ -13,7 +13,7 @@ >> long long simple_strtoll(const char *cp, const char **endp, unsigned int >> base) >> { >> if ( *cp == '-' ) >> - return -simple_strtoull(cp + 1, endp, base); >> + return (-1LL) * simple_strtoull(cp + 1, endp, base); >> return simple_strtoull(cp, endp, base); >> } >> Nor are these two. As to the kind of change here - didn't we deviate applying unary minus to unsigned types? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |