Sergey Bugaev, le mar. 30 janv. 2024 09:38:39 +0300, a ecrit: > On Mon, Jan 29, 2024 at 11:59 PM Samuel Thibault > <samuel.thiba...@gnu.org> wrote: > > Please notably review the RPC part, I really don't know that much about > > mig. > > Some nitpicks inline. Flávio, does what I'm saying below make sense to you? > > > +/* > > + * vm_pages_phys returns information about a region of memory > > + */ > > +kern_return_t vm_pages_phys( > > + host_t host, > > + vm_map_t map, > > + vm_address_t address, > > + vm_size_t size, > > This is supposed to be a number of pages rather than VM region size, > right?
No, I followed the same principle as the vm_allocate etc. calls. In the function itself, count = atop(size). > > + rpc_phys_addr_array_t *pagespp, > > + mach_msg_type_number_t *countp) > > +{ > > + if (host == HOST_NULL) > > + return KERN_INVALID_HOST; > > + if (map == VM_MAP_NULL) > > + return KERN_INVALID_TASK; > > + > > + if (!page_aligned(address)) > > + return KERN_INVALID_ARGUMENT; > > + if (!page_aligned(size)) > > + return KERN_INVALID_ARGUMENT; > > + > > + mach_msg_type_number_t count = atop(size), cur; > > + > > + if (*countp < count) > > + return KERN_INVALID_ARGUMENT; > > It's not an error if the passed-in buffer is smaller than what you're > planning to return. Right, I was lazy here :) and indeed the 256 static count can pose problem. > You're supposed to allocate your own buffer in this case, something like this: Ok, done so, thanks! > OK. My current understanding is that you indeed do not need to > deallocate either the 'vm_map_t map' argument or the task port right > behind it on the success path here. But perhaps it would be clearer > with a comment stating that this is intentional (and not simply > forgotten). This is the case in all RPC calls of the kernel :) Samuel