On 6/7/22 13:14, Warner Losh wrote:
> +static void helper_unlock_iovec(struct target_iovec *target_vec,
> + abi_ulong target_addr, struct iovec *vec,
> + int count, int copy)
> +{
> + for (int i = 0; i < count; i++) {
> + abi_ulong base = tswapal(target_vec[i].iov_base);
> + abi_long len = tswapal(target_vec[i].iov_len);
> + if (len < 0) {
> + /*
> + * Can't really happen: we'll fail to lock if any elements have a
> + * length < 0. Better to fail-safe though.
> + */
> + break;
> + }
I think this is over-complicated, could be fixed by...
> + vec = g_try_new(struct iovec, count);
... using g_try_new0.
I agree. Once I fixed the 'bad_address' issue, I can make this code simpler.
linux-user/syscall.c likely can be simplified as well since this code is a fairly
straight forward copy of that code way back when, it seems...
> + /*
> + * ??? If host page size > target page size, this will result in a value
> + * larger than what we can actually support.
> + * ??? Should we just assert something for new 16k page size on aarch64?
> + */
> + max_len = 0x7fffffff & TARGET_PAGE_MASK;
Use minimum value, I think.
OK. Will update this, and the other suggestions and use the fact that once we
hit an error, we basically don't do anything to the zero'd frame, which makes it
even simpler (and still, I think) correct. I'll send V2 kinda quickly as a result after
my smoke test...
Warner