Re: [Xen-devel] [PATCH v8.1 17/27] xsplice: Add support for bug frames.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 04/14/16 12:02 AM >>>
>+bool_t is_patch(const void *ptr)
>+{
>+    struct payload *data;

You guess it: const.

>+    /*
>+     * No locking since this list is only ever changed during apply or revert
>+     * context.
>+     */

What if you crash while applying or reverting a patch? Is the list update at
least done such that the (then nested) traversal remains safe?

>@@ -482,6 +508,30 @@ static int prepare_payload(struct payload *payload,
     >region->start = (unsigned long)payload->text_addr;
     >region->end = (unsigned long)(payload->text_addr + payload->text_size);
 >
>+    /* Optional sections. */
>+    for ( i = 0; i < BUGFRAME_NR; i++ )
>+    {
>+        char str[14];
>+
>+        snprintf(str, sizeof(str), ".bug_frames.%u", i);
>+        sec = xsplice_elf_sec_by_name(elf, str);
>+        if ( !sec )
>+            continue;
>+
>+        if ( sec->sec->sh_size &&
>+             (sec->sec->sh_size % sizeof(*region->frame[i].bugs)) )

The left side of the && seems pointless.

>+        {
>+            dprintk(XENLOG_ERR, XSPLICE "%s: Wrong size of .bug_frames.%u!\n",
>+                    elf->name, i);
>+            return -EINVAL;
>+        }
>+
>+        region->frame[i].bugs = sec->load_addr;
>+        if ( sec->sec->sh_size)
>+            region->frame[i].n_bugs = sec->sec->sh_size /
>+                                      sizeof(*region->frame[i].bugs);

As does the conditional here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel