| Message ID | c3323f6b-4af2-0bfb-9399-e529952e378e@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 24 Aug 2016 20:36:26 +0200 SF Markus Elfring <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Wed, 24 Aug 2016 19:45:23 +0200 > > * A multiplication for the size determination of a memory allocation > indicated that an array data structure should be processed. > Thus reuse the corresponding function "kmalloc_array". > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > > This issue was detected also by using the Coccinelle software. Do you have the scripts you use published somewhere? > > * Replace the specification of data structures by pointer dereferences > to make the corresponding size determination a bit safer according to > the Linux coding style convention. > > * Delete the local variable "size" which became unnecessary with > this refactoring. I think your description is a bit on the verbose side, but not enough to gripe more. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > > v2: Rebased on source files from "Linux next-20160824". > Advices were integrated from source code review. > > arch/s390/kvm/guestdbg.c | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> This issue was detected also by using the Coccinelle software. > > Do you have the scripts you use published somewhere? Not yet. I hope that I can clarify a few more implementation details around my evolving scripts for the semantic patch language. I guess that this approach can be published with a higher confidence in the near future. Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> This issue was detected also by using the Coccinelle software. > > Do you have the scripts you use published somewhere? I would like to add another view for the corresponding software development. The complete answer depends also on the kind of "scripts" you are looking for. Would you like to clarify any details a bit more here? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Aug 2016 19:34:29 +0200 SF Markus Elfring <elfring@users.sourceforge.net> wrote: > >> This issue was detected also by using the Coccinelle software. > > > > Do you have the scripts you use published somewhere? > > I would like to add another view for the corresponding software development. > > The complete answer depends also on the kind of "scripts" > you are looking for. Would you like to clarify any details > a bit more here? You obviously run some kind of semantic patching. It would really help ease review if you could publish the semantic patches that generate your patches - that is probably more helpful in review than just posting the generated patches. And it does not need to be "complete", I'd think everyone on the cc: list here is able to handle a cocchinelle patch, for example. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> You obviously run some kind of semantic patching. Yes. - I developed some software for search patterns I became interested in. > It would really help ease review if you could publish the semantic patches > that generate your patches This is reasonable. > - that is probably more helpful in review than just posting the generated patches. Which of the available scripts (or SmPL source files) would you like to discuss further? > And it does not need to be "complete", I would feel more comfortable with publishing further scripts here when I can become more confident about relevant safety checks in this software area. > I'd think everyone on the cc: list here is able to handle a cocchinelle patch, > for example. Interesting view … I did not expect this kind of expertise by default. Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Aug 2016, SF Markus Elfring wrote: > >> This issue was detected also by using the Coccinelle software. > > > > Do you have the scripts you use published somewhere? > > I would like to add another view for the corresponding software development. > > The complete answer depends also on the kind of "scripts" > you are looking for. Would you like to clarify any details > a bit more here? I imagine that she is asking for: @@ expression e1,e2,e3; @@ - kmalloc(e1 * e2, e3) + kmalloc_array(e1, e2, e3) Or some close variant. It seems pretty straightforward to provide and can help orient the reader to what is going on. julia -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Or some close variant.
I have got more script variants evolving in my software collection.
There are further approaches available from various contributors,
aren't there?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Aug 2016, SF Markus Elfring wrote: > > Or some close variant. > > I have got more script variants evolving in my software collection. > > There are further approaches available from various contributors, > aren't there? What she is asking for is a concise and precise decription of what you have done. If you have some other variants, eg controlling where the sizeof argument is (left or right of *), you don't necessarily have to include it in the patch, if such a rule was not used for the specific patch anyway. julia > > Regards, > Markus > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Aug 2016 14:23:35 -0400 (EDT) Julia Lawall <julia.lawall@lip6.fr> wrote: > On Thu, 25 Aug 2016, SF Markus Elfring wrote: > > > > Or some close variant. > > > > I have got more script variants evolving in my software collection. > > > > There are further approaches available from various contributors, > > aren't there? > > What she is asking for is a concise and precise decription of what you > have done. If you have some other variants, eg controlling where the > sizeof argument is (left or right of *), you don't necessarily have to > include it in the patch, if such a rule was not used for the specific > patch anyway. *nod* If I see a patch that says "I've run the following cocchinelle patch to perform $TRANSFORMATION, and here's the result", I can be reasonably sure that the result will be what is intended to be changed in the first place (and I can assess whether the change makes sense at all.) If I see only the resulting patch, I won't know whether you have performed the changes manually (and possibly introduced bugs, as happens to all of us.) Moreover, a good semantic patch is useful to others as well and might even be reused in other contexts that have similar requirements. You really lose value if you don't publish them. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/s390/kvm/guestdbg.c b/arch/s390/kvm/guestdbg.c index d1f8241..70b71ac 100644 --- a/arch/s390/kvm/guestdbg.c +++ b/arch/s390/kvm/guestdbg.c @@ -206,7 +206,7 @@ static int __import_wp_info(struct kvm_vcpu *vcpu, int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg) { - int ret = 0, nr_wp = 0, nr_bp = 0, i, size; + int ret = 0, nr_wp = 0, nr_bp = 0, i; struct kvm_hw_breakpoint *bp_data = NULL; struct kvm_hw_wp_info_arch *wp_info = NULL; struct kvm_hw_bp_info_arch *bp_info = NULL; @@ -216,14 +216,17 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, else if (dbg->arch.nr_hw_bp > MAX_BP_COUNT) return -EINVAL; - size = dbg->arch.nr_hw_bp * sizeof(struct kvm_hw_breakpoint); - bp_data = kmalloc(size, GFP_KERNEL); + bp_data = kmalloc_array(dbg->arch.nr_hw_bp, + sizeof(*bp_data), + GFP_KERNEL); if (!bp_data) { ret = -ENOMEM; goto error; } - if (copy_from_user(bp_data, dbg->arch.hw_bp, size)) { + if (copy_from_user(bp_data, + dbg->arch.hw_bp, + sizeof(*bp_data) * dbg->arch.nr_hw_bp)) { ret = -EFAULT; goto error; } @@ -241,17 +244,19 @@ int kvm_s390_import_bp_data(struct kvm_vcpu *vcpu, } } - size = nr_wp * sizeof(struct kvm_hw_wp_info_arch); - if (size > 0) { - wp_info = kmalloc(size, GFP_KERNEL); + if (nr_wp > 0) { + wp_info = kmalloc_array(nr_wp, + sizeof(*wp_info), + GFP_KERNEL); if (!wp_info) { ret = -ENOMEM; goto error; } } - size = nr_bp * sizeof(struct kvm_hw_bp_info_arch); - if (size > 0) { - bp_info = kmalloc(size, GFP_KERNEL); + if (nr_bp > 0) { + bp_info = kmalloc_array(nr_bp, + sizeof(*bp_info), + GFP_KERNEL); if (!bp_info) { ret = -ENOMEM; goto error;