On 18/11/2022 10:44, Beata Michalska wrote:
@@ -3138,7 +3333,28 @@ int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args) while (isalnum(*fmt)) fmt++; break; +#ifdef __CHERI__
case FORMAT_TYPE_CAPABILITY:/** Capabilities shall be handled now: subject to* required alignment, plus exercising extra caution* when storing the capability in the buffer directly.* Otherwise breaking down the capability* to extract the information requested (format)* would be rather counter-productive.I'm still not 100% sure what the last sentence refers to, is it that we could have just saved the capability if not for the two reasons above?
I agree it's not clearly laid out. An alternative to handling capabilities here (as per formatting), or storing it in the buffer, was to break it into tag, metadata and address values, and store those in the buffer instead.
Ah right I get it now!
Dropping that line in v4.
That's probably better yes (otherwise you'd have to explain what you mean more specifically and that may not really be worth it here).
Kevin
*/str = capability(fmt, str, end,va_arg(args, void * __capability),spec);if (str + 1 < end)*str++ = '\0';elseend[-1] = '\0'; /* Must be null terminated */while (isalnum(*fmt))fmt++;
break;+#endif default: switch (spec.type) { @@ -3319,7 +3535,22 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf) fmt++; break; } +#ifdef __CHERI__
case FORMAT_TYPE_CAPABILITY: {long length = strlen(args);if (str < end) {memcpy(str, args, min(length, (end - str)));I think you still have the second issue I mentioned in v2, i.e. the length you use in the two lines below should be the same length memcpy() is passed here, like in FORMAT_TYPE_PTR.
What FORMAT_TYPE_PTR is doing is the same already:
FORMAT_TYPE_PTR FORMAT_TYPE_CAPABILITY
str += len; -> str += length; args += len + 1; -> args += length + 1;
where: len = copy = strlen(args); -> length = strlen(args);
and with:
memcpy(str, args, copy); -> memcpy(str, args, min(length, (end - str)));
The only difference between the two being:
/* copy = strlen(args); */ if (copy > end - str) copy = end - str; => /* copy == min(length, (end - str)) */ memcpy(str, args, copy); memcpy(str, args, min(length, (end - str)));
Note that the 'str' will be updated based on the expected, not actual length. As per doc comments for bstr_printf:
- The return value is the number of characters which would
- be generated for the given input ....
Or am I still missing smth (which might be the case)?
I'm sorry you're obviously right, your code clearly does the same thing as FORMAT_TYPE_PTR, I misread the latter. Nothing to change on that front, sorry for the noise.
Kevin