Re: [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer.

[Dave Young]

On 06/30/16 at 01:42pm, Thiago Jung Bauermann wrote:
> Am Donnerstag, 30 Juni 2016, 12:49:44 schrieb Thiago Jung Bauermann:
> > Am Donnerstag, 30 Juni 2016, 11:07:00 schrieb Dave Young:
> > > On 06/29/16 at 06:18pm, Thiago Jung Bauermann wrote:
> > > > I'm not following. The IMA buffer patchset doesn't use
> > > > kexec_locate_mem_hole nor struct kexec_buf.
> > > 
> > > It does not use kexec_locate_mem_hole, but the buffer being passed is
> > > very similar to a kexec_buf struct, no?
> > 
> > If what you're saying is that the arguments passed to
> > kexec_add_handover_buffer in the IMA buffer patchset are very similar to
> > the arguments passed to kexec_add_buffer then yes, it's true.
> > 
> > > So you may refactor kexec_add_buffer and your new function to pass only
> > > kimage and a kbuf, it will be better than passing all those arguments
> > > separately.
> > 
> > To be honest I think struct kexec_buf is an implementation detail inside
> > kexec_locate_mem_hole, made necessary because the callback functions it
> > uses need to access its arguments. Callers of kexec_locate_mem_hole,
> > kexec_add_handover_buffer and kexec_add_buffer shouldn't need to know it
> > exists.
> 
> Elaborating a bit more: the argument list for these three functions are 
> equal or similar because kexec_add_handover_buffer uses kexec_add_buffer, 
> which uses kexec_locate_mem_hole.
> 
> It could be beneficial to have a struct to collect the arguments to these 
> functions if someone using one of them would be likely to use another one 
> with the same arguments. In that case, you set up kexec_buf once and then 
> just pass it whenever you need to call one of those functions.
> 
> But that is unlikely to happen. A user of the kexec API will need to use 
> only one of these functions with a given set of arguments, so they don't 
> gain anything by setting up a struct.
> 
> Syntactically, I also don't think it's clearer to set struct members instead 
> of simply passing arguments to a function, even if the argument list is 
> long.

Sorry, I'm not sure I get your points but the long argument list really looks ugly,
since you are introducing more callbacks I still think a cleanup is necessary.

kexec_buffer struct is pretty fine to be a abstract of all these buffers.

Thanks
Dave