Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 11, 2015 at 04:00:19PM +0200, Jan Kara wrote:
> > > +int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> > > +		     bool write, bool force, struct frame_vector *vec)
> > > +{
> > > +	struct mm_struct *mm = current->mm;
> > > +	struct vm_area_struct *vma;
> > > +	int ret = 0;
> > > +	int err;
> > > +	int locked = 1;
> > > +
> > 
> > bool locked.
>   It cannot be bool. It is passed to get_user_pages_locked() which expects
> int *.
> 

My bad.

> > > +int frame_vector_to_pages(struct frame_vector *vec)
> > > +{
> > 
> > I think it's probably best to make the relevant counters in frame_vector
> > signed and limit the maximum possible size of it. It's still not putting
> > any practical limit on the size of the frame_vector.
>
>   I don't see a reason why counters in frame_vector should be signed... Can
> you share your reason?  I've added a check into frame_vector_create() to
> limit number of frames to INT_MAX / sizeof(void *) / 2 to avoid arithmetics
> overflow. Thanks for review!
> 

Only that the return value of frame_vector_to_pages() returns int where
as the potential range that is converted is unsigned int. I don't think
there are any mistakes dealing with signed/unsigned but I don't see any
advantage of using unsigned either and limiting it to INT_MAX either.
It's not a big deal.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux