[stgt] [PATCH 02/13] mem_copy_n32: safe memcpy, accumulates copied count, tracks remaining space

[FUJITA Tomonori]

On Mon, 11 Feb 2013 18:24:05 +0900 (JST)
FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp> wrote:

> On Mon, 11 Feb 2013 11:16:23 +0200
> Alexander Nezhinsky <nezhinsky at gmail.com> wrote:
> 
>> On Mon, Feb 11, 2013 at 10:39 AM, FUJITA Tomonori
>> <fujita.tomonori at lab.ntt.co.jp> wrote:
>> 
>>> Alexander Nezhinsky <nezhinsky at gmail.com> wrote:
>>>> For example, please see build_mode_page() usage pattern.
>> 
>>> I already read that code. Why you can't simply do something like:
>>> actual_len = mem_copy_n32(data, hdr, hdr_size,
>>>                           remain_len);
>>> *avail_len += hdr_size;
>>>
>>> mem_copy_n32 unconditionally adds src_len to avail_len. I don't see
>>> any point of doing that in mem_copy_n32().
>> 
>> Well, i can do it either way.
>> 
>> Then we are left with this:
>>>> 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_memcpy(uint8_t *dst, uint32_t dst_len, uint8_t *src, uint32_t src_len)
> 
> Looks more normal.
> 
> This is not about spc only? If so, I think that it's should be a
> different name.

uint8_t *dst, uint32_t *dst_remain_len, uint8_t *src, uint32_t src_len


> 
>> As such changes affect the entire patchset, I'd prefer to finalize all
>> decisions before resending, especially regrading names, locations etc.
>> 
>> Is it ok as appears above?
>> 
>> Also regarding the other function:
>>>> what about safe_set_byte(...) ?
>>>> i'd leave it in utils.c, but we can move to spc.c as well
>> 
>> Do you have other comments?
> 
> My question is that there is any possiblity that this is used by other
> than mod_sense? If it's only about mod_sense, then please use more
> explict name.
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html