On 01/10/18 11:14, Jan Beulich wrote:
>>>> On 01.10.18 at 12:02, <andrew.coop...@citrix.com> wrote:
>> On 01/10/18 10:08, Jan Beulich wrote:
>>>>>> On 28.09.18 at 19:22, <andrew.coop...@citrix.com> wrote:
>>>> +static char *print_domain(char *str, char *end, const struct domain *d)
>>>> +{
>>>> + const char *name = NULL;
>>>> +
>>>> + /* Some debugging may have an optionally-NULL pointer. */
>>>> + if ( unlikely(!d) )
>>>> + return string(str, end, "NULL", -1, -1, 0);
>>>> +
>>>> + if ( str < end )
>>>> + *str = 'd';
>>>> +
>>>> + switch ( d->domain_id )
>>>> + {
>>>> + case DOMID_IO: name = "[IO]"; break;
>>>> + case DOMID_XEN: name = "[XEN]"; break;
>>>> + case DOMID_COW: name = "[COW]"; break;
>>>> + case DOMID_IDLE: name = "[IDLE]"; break;
>>> default: ASSERT_UNREACHABLE();
>>>
>>> ?
>> No - specifically not in this case.
>>
>> This path is used when printing crash information, and falling back to a
>> number is better behaviour than falling into an infinite loop,
>> overflowing the primary stack, then taking a #DF (which escalates to
>> triple fault on AMD), without printing anything useful.
> Ah, good point. Perhaps worth a brief comment instead of a "default:"
> then?
This incremental diff?
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index cd2d7d9..b0ff00c 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -279,6 +279,11 @@ static char *print_domain(char *str, char *end,
const struct domain *d)
case DOMID_XEN: name = "[XEN]"; break;
case DOMID_COW: name = "[COW]"; break;
case DOMID_IDLE: name = "[IDLE]"; break;
+ /*
+ * In principle, we could ASSERT_UNREACHABLE() in the default case.
+ * However, this path is used to print out crash information, which
+ * risks recursing infinitely and not printing any useful
information.
+ */
}
if ( str < end )
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
