On Mon, Feb 11, 2013 at 2:07 AM, FUJITA Tomonori
<fujita.tomonori@xxxxxxxxxxxxx> wrote:
>> From: Alexander Nezhinsky <nezhinsky@xxxxxxxxx>
>>
>> + * Copy up to src_len bytes from src,
>> + * not exceeding *remain_len bytes available at dst.
>> + * Return actually copied length.
>> + * Reflect decreased space in *remain_len (mandatory).
>> + * Accumulate copied length in *avail_len (optional).
>> + */
>> +int mem_copy_n32(uint8_t *dst, uint8_t *src, uint32_t src_len,
>> + uint32_t *avail_len, uint32_t *remain_len)
> Something like the following looks more reasonable?
> scsi_memcpy(dst, str_len, src, src_len)
>
> I'm not sure about 'avail_len'. src_len is const so it doesn't make
> sense to calculate avail_len inside this function.
avail_len (or its equivalent under alternatve name) is necessary, as it is
sometimes used to accumulate the available buffer length through a
sequence of nested calls.
For example, please see build_mode_page() usage pattern.
The source of confusion (also referred to from a couple of your other RE: mails)
seems to be the parameter and variable names: avail_len passed as the value
of src_len parameter, and NULL passed for *avail_len parameter.
What about having the function split int two (eliminates occasional NULL arg)
and avail_len renamed to accum_len in the special "accumulating" version?
int spc_memcpy(uint8_t *dst, uint8_t *src, uint32_t src_len, uint32_t
*remain_len)
{
int copy_len = min_t(uint32_t, *remain_len, src_len);
if (copy_len) {
memcpy(dst, src, copy_len);
*remain_len -= copy_len;
}
return copy_len;
}
int spc_accum_memcpy(uint8_t *dst, uint8_t *src, uint32_t src_len,
uint32_t *remain_len, uinnt32_t *accum_len)
{
spc_memcpy(dst, src, src_len, remain_len);
*accum_len += src_len;
}
so for example in report_opcodes_all() the call would be :
actual_len = spc_memcpy(scsi_get_in_buffer(cmd), buf, avail_len, &alloc_len);
while in spc_pr_read_keys() it would be :
actual_len += spc_accum_memcpy(&buf[actual_len], (uint8_t *)®_key, 8,
&remain_len, /* update
remaining len here */
&avail_len); /* accumulate
available len here */
>> +void set_byte_n32(int val, uint8_t *dst, uint32_t index, uint32_t
>> max_len)
>> +{
>> + if (index < max_len)
>> + dst[index] = (uint8_t)val;
>> +}
>
> This function could be used by other than mod_sense? If not, I prefer
> to have more explicit function name in spc.c
what about safe_set_byte(...) ?
i'd leave it in utils.c, but we can move to spc.c as well
What do you think?
Alexander
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
