Re: [PATCH v3 1/3] lib: bitmap: introduce bitmap_print_to_buf

[Jonathan Cameron]

On Thu, 3 Jun 2021 14:11:16 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> On Thu, Jun 03, 2021 at 06:33:25PM +0800, tiantao (H) wrote:
> > 在 2021/6/3 17:50, Andy Shevchenko 写道:  
> > > On Thu, Jun 03, 2021 at 05:22:40PM +0800, Tian Tao wrote:  
> > > > New API bitmap_print_to_buf() with bin_attribute to avoid maskp
> > > > exceeding PAGE_SIZE. bitmap_print_to_pagebuf() is a special case
> > > > of bitmap_print_to_buf(), so in bitmap_print_to_pagebuf() call
> > > > bitmap_print_to_buf().  
> 
> ...
> 
> > > > + * @count: the maximum number of bytes to print  
> 
> > > > +	size = memory_read_from_buffer(buf, count, &off, data, strlen(data) + 1);  
> > > Are you sure you have put parameters in the correct order?  
> > 
> > yes, I already test it.
> > 
> > ssize_t memory_read_from_buffer(void *to, size_t count, loff_t *ppos,
> >                                 const void *from, size_t available)  
> 
> Have you read the meaning of count and available?
> Please, double check that they are filled with correct values.

Ok, I don't get this one either so can you give us more of a hint?

/**
 * memory_read_from_buffer - copy data from the buffer
 * @to: the kernel space buffer to read to
 * @count: the maximum number of bytes to read
 * @ppos: the current position in the buffer
 * @from: the buffer to read from
 * @available: the size of the buffer
 *
 * The memory_read_from_buffer() function reads up to @count bytes from the
 * buffer @from at offset @ppos into the kernel space address starting at @to.
 *
 * On success, the number of bytes read is returned and the offset @ppos is
 * advanced by this number, or negative value is returned on error.
 **/

These docs do end up rather confusing by using the term buffer for multiple things
but taking what is passed in.

Count is the maximum in the sense of how many bytes we are requesting are read
which indeed should be count here as that reflects what userspace asked for.

Avail is the size of the buffer we are reading from.  Now that's slightly
ambiguous in the docs in the sense of 'buffer' could mean the to buffer or
the from buffer.  However, I'd assume count is definitely <= size of the space
after address to in the to buffer, so I would assume that means available
is the size of the from buffer.  Here that is strlen() + 1, so looks fine.

This interpretation also lines up with the implementation.

So what are we missing?

Jonathan

> 
> > > I guess you have to provide the test case(s).  
>