xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <ian.campbell@citrix.com>
To: Joshua Otto <jtotto@uwaterloo.ca>, xen-devel@lists.xen.org
Cc: hjarmstr@uwaterloo.ca, czylin@uwaterloo.ca
Subject: Re: Taking on a Xen development project
Date: Mon, 14 Dec 2015 11:08:43 +0000	[thread overview]
Message-ID: <1450091323.16856.24.camel@citrix.com> (raw)
In-Reply-To: <20151212220749.GA8820@personal>

On Sat, 2015-12-12 at 17:07 -0500, Joshua Otto wrote:
> On Fri, Dec 11, 2015 at 01:52:41PM +0000, Ian Campbell wrote:
> > Cool! Just to be clear, you are looking for one project for the 3 of
> > you to
> > work on as a group (vs 3 individual projects), is that right?
> 
> Yes, that's right.
> 
> > It's been a while since there has been a scan run, I did one yesterday but
> > it is taking an unusually long time to get the results back. Hopefully
> > we'll have an up to date set of defects early next week and I can have a
> > scrobble around for some interesting ones for you guys to take a look at.
> 
> That would be perfect, thanks!

Results are in. I've cherry-picked a few of the new issues below. I've not
checked carefully for false +ves.

Not a great deal of massive thrills in there, but some one liners etc to
dip your toes in I guess.

Ian.
________________________________________________________________________________________________________
*** CID 1343310:  Code maintainability issues  (UNUSED_VALUE)
/xen/arch/x86/hvm/svm/intr.c: 95 in svm_enable_intr_window()
89                 struct vmcb_struct *gvmcb = nv->nv_vvmcx;
90     
91                 /* check if l1 guest injects interrupt into l2 guest via vintr.
92                  * return here or l2 guest looses interrupts, otherwise.
93                  */
94                 ASSERT(gvmcb != NULL);
>>>     CID 1343310:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value from "vmcb_get_vintr(gvmcb)" to "intr" here, but that stored value is overwritten before it can be used.
95                 intr = vmcb_get_vintr(gvmcb);
96                 if ( intr.fields.irq )
97                     return;
98             }
99         }
100     
________________________________________________________________________________________________________
*** CID 1343309:  Control flow issues  (UNREACHABLE)
/tools/libxl/libxl.c: 5575 in libxl_get_scheduler()
5569     {
5570         libxl_scheduler sched, ret;
5571         GC_INIT(ctx);
5572         if ((ret = xc_sched_id(ctx->xch, (int *)&sched)) != 0) {
5573             LOGE(ERROR, "getting domain info list");
5574             return ERROR_FAIL;
>>>     CID 1343309:  Control flow issues  (UNREACHABLE)
>>>     This code cannot be reached: "libxl__free_all(gc);".
5575             GC_FREE;
5576         }
5577         GC_FREE;
5578         return sched;
5579     }
5580     

As well as putting GC_FREE in the right place this function could be
reworked to follow the recommendations in tools/libxl/CODING_STYLE.

** CID 1343307:    (RESOURCE_LEAK)
/tools/libxl/libxl_dm.c: 746 in libxl__dm_runas_helper()
/tools/libxl/libxl_dm.c: 748 in libxl__dm_runas_helper()
/tools/libxl/libxl_dm.c: 749 in libxl__dm_runas_helper()


________________________________________________________________________________________________________
*** CID 1343307:    (RESOURCE_LEAK)
/tools/libxl/libxl_dm.c: 746 in libxl__dm_runas_helper()
740             ret = getpwnam_r(username, &pwd, buf, buf_size, &user);
741             if (ret == ERANGE) {
742                 buf_size += 128;
743                 continue;
744             }
745             if (ret != 0)
>>>     CID 1343307:    (RESOURCE_LEAK)
>>>     Variable "buf" going out of scope leaks the storage it points to.
746                 return ERROR_FAIL;
747             if (user != NULL)
748                 return 1;
749             return 0;
750         }
751     }
/tools/libxl/libxl_dm.c: 748 in libxl__dm_runas_helper()
742                 buf_size += 128;
743                 continue;
744             }
745             if (ret != 0)
746                 return ERROR_FAIL;
747             if (user != NULL)
>>>     CID 1343307:    (RESOURCE_LEAK)
>>>     Variable "buf" going out of scope leaks the storage it points to.
748                 return 1;
749             return 0;
750         }
751     }
752     
753     static int libxl__build_device_model_args_new(libxl__gc *gc,
/tools/libxl/libxl_dm.c: 749 in libxl__dm_runas_helper()
743                 continue;
744             }
745             if (ret != 0)
746                 return ERROR_FAIL;
747             if (user != NULL)
748                 return 1;
>>>     CID 1343307:    (RESOURCE_LEAK)
>>>     Variable "buf" going out of scope leaks the storage it points to.
749             return 0;
750         }
751     }
752     
753     static int libxl__build_device_model_args_new(libxl__gc *gc,
754                                             const char *dm, int guest_domid,

*** CID 1343302:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
/xen/drivers/char/ns16550.c: 916 in pci_uart_config()
910     
911                             p = uart_config[i].param;
912                             /*
913                              * Force length of mmio region to be at least
914                              * 8 bytes times (1 << reg_shift)
915                              */
>>>     CID 1343302:  Integer handling issues  (OVERFLOW_BEFORE_WIDEN)
>>>     Potentially overflowing expression "1 << uart_param[p].reg_shift" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "u64" (64 bits, unsigned).
916                             if ( size < (0x8 * (1 << uart_param[p].reg_shift)) )
917                                 continue;
918     
919                             if ( bar_idx >= uart_param[p].max_bars )
920                                 continue;
921     
________________________________________________________________________________________________________
*** CID 1343301:  Incorrect expression  (NO_EFFECT)
/xen/common/sched_credit.c: 1795 in csched_dump_pcpu()
1789     csched_dump_pcpu(const struct scheduler *ops, int cpu)
1790     {
1791         struct list_head *runq, *iter;
1792         struct csched_private *prv = CSCHED_PRIV(ops);
1793         struct csched_pcpu *spc;
1794         struct csched_vcpu *svc;
>>>     CID 1343301:  Incorrect expression  (NO_EFFECT)
>>>     Assignment operation "lock = lock" has no effect.
1795         spinlock_t *lock = lock;
1796         unsigned long flags;
1797         int loop;
1798     #define cpustr keyhandler_scratch
1799     
1800         /*

________________________________________________________________________________________________________
*** CID 1343299:  Incorrect expression  (MIXED_ENUMS)
/tools/libxl/libxl_psr.c: 313 in libxl_psr_cat_set_cbm()
307             goto out;
308         }
309     
310         libxl_for_each_set_bit(socketid, *target_map) {
311             if (socketid >= nr_sockets)
312                 break;
>>>     CID 1343299:  Incorrect expression  (MIXED_ENUMS)
>>>     Mixing enum types "enum libxl_psr_cbm_type" and "enum xc_psr_cat_type" for "type".
313             if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socketid, cbm)) {
314                 libxl__psr_cat_log_err_msg(gc, errno);
315                 rc = ERROR_FAIL;
316             }
317         }
318     
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2015-12-14 11:08 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-04 20:26 Taking on a Xen development project jtotto
2015-12-10 10:59 ` Wei Liu
2015-12-10 17:23 ` Andrew Cooper
2015-12-12  2:19   ` Yang Hongyang
2015-12-12 22:30     ` Joshua Otto
2015-12-12 23:02       ` Andrew Cooper
2015-12-14 22:49         ` Joshua Otto
2015-12-11 13:52 ` Ian Campbell
2015-12-12 22:07   ` Joshua Otto
2015-12-14 11:08     ` Ian Campbell [this message]
2015-12-14 22:59       ` Joshua Otto
2015-12-15 15:48         ` Ian Campbell
2015-12-28  5:16       ` Coverity tidying Joshua Otto
2015-12-28  5:16         ` [PATCH 1/5] libxl: tidy libxl_get_scheduler() according to CODING_STYLE Joshua Otto
2016-01-04 16:23           ` Ian Campbell
2016-01-05  8:20             ` Dario Faggioli
2016-01-19  5:57               ` [PATCH v2 " Chester Lin
2016-01-19  9:14                 ` Dario Faggioli
2016-01-19 11:28                 ` Wei Liu
2016-01-19 11:35                   ` Ian Campbell
2015-12-28  5:16         ` [PATCH 2/5] libxl: make GC_FREE reachable in libxl_get_scheduler() Joshua Otto
2016-01-04 16:29           ` Ian Campbell
2016-01-05  8:49             ` Dario Faggioli
2016-01-05 11:16               ` Ian Campbell
2016-01-19  5:57                 ` [PATCH v2 " Chester Lin
2016-01-19  9:08                   ` Dario Faggioli
2016-01-19 14:15                   ` Ian Jackson
2015-12-28  5:16         ` [PATCH 3/5] ns16550: widen an integer constant for Coverity Joshua Otto
2016-01-04 16:36           ` Ian Campbell
2016-01-06  9:26             ` Jan Beulich
2016-01-19  5:57               ` [PATCH v2 3/5] n16550: add sanity check for reg_shift Chester Lin
2016-01-19 13:32                 ` Jan Beulich
2016-01-25  0:41                   ` czylin
2015-12-28  5:16         ` [PATCH 4/5] credit: remove pointless local variable initialization Joshua Otto
2015-12-28  5:16         ` [PATCH 5/5] libxl: Add explicit cast to libxl_psr_cat_set_cbm Joshua Otto
2016-01-04 16:40           ` Ian Campbell
2016-01-19  5:58             ` [PATCH v2 " Chester Lin
2016-01-19  8:34               ` Dario Faggioli
2016-01-19 14:06               ` Ian Jackson
2016-01-19 14:21                 ` Ian Campbell
2016-01-19 14:28                   ` Dario Faggioli
2016-01-19 14:33                     ` Ian Jackson
2016-01-19 14:31                   ` George Dunlap
2016-01-19 14:31                 ` Ian Campbell
2016-01-19 14:35                   ` Ian Jackson
2017-01-12 18:08                     ` George Dunlap
2017-01-13  9:05                       ` Dario Faggioli
2015-12-28  9:34         ` Coverity tidying Andrew Cooper
2016-01-01  3:14           ` [PATCH] svm: rephrase local variable use for Coverity Joshua Otto
2016-01-06 13:24             ` Jan Beulich
2016-01-06 14:33               ` Boris Ostrovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1450091323.16856.24.camel@citrix.com \
    --to=ian.campbell@citrix.com \
    --cc=czylin@uwaterloo.ca \
    --cc=hjarmstr@uwaterloo.ca \
    --cc=jtotto@uwaterloo.ca \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).