Re: [PATCH net-next v17 03/14] netdev: support binding dma-buf to netdevice

>> … >>> +++ b/include/net/devmem.h >>> @@ -0,0 +1,115 @@ >> … >>> +#ifndef _NET_DEVMEM_H >>> +#define _NET_DEVMEM_H >> … >> >> I suggest to omit leading underscores from such identifiers. >> https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier >> >

Re: [PATCH net-next v17 03/14] netdev: support binding dma-buf to netdevice

… > +++ b/include/net/devmem.h > @@ -0,0 +1,115 @@ … > +#ifndef _NET_DEVMEM_H > +#define _NET_DEVMEM_H … I suggest to omit leading underscores from such identifiers. https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier Regards, Markus

Re: [PATCH v5] tools/testing: Fix the wrong format specifier

> The format specifier in fprintf is "%u", that "%u" should use > unsigned int type instead. * Please improve the change description with imperative wordings. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10#n94 * Woul

Re: [PATCH] selftests/bpf:fix a resource leak in main()

> The requested resources should be closed before return in main(), otherwise > resource leak will occur. Add a check of cg_fd before close(). > > Fixes: 435f90a338ae ("selftests/bpf: add a test case for sock_ops perf-event > notification") > Signed-off-by: Ma Ke Please reconsider such informati

Re: [PATCH v3] selftests/capabilities: Fix possible file leak in copy_fromat_to

>… openat() and open() initialize > 'from' and 'to', and only 'from' validated with 'if' statement. Why do you find such information helpful? > If the > initialization of variable 'to' fails,

Re: [PATCH v2] selftests/capabilities: Fix possible file leak in copy_fromat_to

>… openat() and open() initialize > 'from' and 'to', and only 'from' validated with 'if' statement. Why do you find such information helpful? > If the > initialization of variable 'to' fails,

Re: [PATCH] selftests/capabilities: Fix possible file leak in copy_fromat_to

>… openat() and open() initialize > 'from' and 'to', and only 'from' validated with 'if' statement. Why do find such information helpful? > If the > initialization of variable 'to' fails, The

Re: [PATCH net-next v12 03/13] netdev: support binding dma-buf to netdevice

… > +++ b/net/core/netdev-genl.c … > int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info) > { … > + rtnl_lock(); > + > + netdev = __dev_get_by_index(genl_info_net(info), ifindex); … > +err_unlock: > + rtnl_unlock(); > + return err; > } … Would you become inter

Re: [PATCH bpf-next v2 1/4] selftests/bpf: Add some null pointer checks

> There is a 'malloc' call, which can be unsuccessful. two calls? > This patch will add the malloc failure checking … Please use imperative wordings for improved change descriptions also in your patches. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/

Re: [PATCH] selftests: cgroup: remove redundant addition of memory controller

> This is already done in main. Please improve this change description. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9-rc6#n45 Will the tag “Fixes” become relevant here (besides an imperative wording)? Regards, Markus

Re: kselftest: arm64: Add a null pointer check

> > There is a 'malloc' call, which can be unsuccessful. > > This patch will add the malloc failure checking > > to avoid possible null dereference and give more information > > about test fail reasons. > > Applied to arm64 (for-next/selftests), thanks! > > [1/1] kselftest: arm64: Add a null pointe

Re: [PATCH bpf-next 4/4] selftests/bpf: Add a null pointer check for the serial_test_tp_attach_query

… > Add the malloc failure checking to avoid possible null > dereference. … How do you think about to use the following wording variant? Add a return value check so that a null pointer dereference will be avoided after a memory allocation failure. Would you like to add the tag “Fixes” acc

Re: [PATCH bpf-next 3/4] selftests/bpf: Add a null pointer check for the load_btf_spec

… > Add the malloc failure checking to avoid possible null > dereference. … How do you think about the following wording variant? Add a return value check so that a null pointer dereference will be avoided after a memory allocation failure. Would you like to add the tag “Fixes” accordingl

Re: [PATCH bpf-next 2/4] selftests/bpf/sockopt: Add a null pointer check for the run_test

… > This patch will add the malloc failure checking … * Please use a corresponding imperative wording for the change description. * Would you like to add the tag “Fixes” accordingly? Regards, Markus

Re: [PATCH bpf-next 1/4] selftests/bpf: Add some null pointer checks

… > This patch will add the malloc failure checking … * Please use a corresponding imperative wording for the change description. * Would you like to add the tag “Fixes” accordingly? … > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -582,6 +582,11 @@ int compare_stack_ips(int smap_fd, int

Re: [PATCH] KVM: selftests: Add 'malloc' failure check in config_name

… > This patch will add the malloc failure checking … * Please use a corresponding imperative wording for the change description. * Would you like to add the tag “Fixes” accordingly? … > +++ b/tools/testing/selftests/kvm/get-reg-list.c > @@ -66,6 +66,7 @@ static const char *config_name(struct v

Re: [PATCH] KVM: selftests: Add 'malloc' failure check in test_vmx_nested_state

… > This patch will add the malloc failure checking … * Please use a corresponding imperative wording for the change description. * Would you like to add the tag “Fixes” accordingly? … > +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c > @@ -91,6 +91,7 @@ void test_vmx_neste

Re: [PATCH] kselftest: arm64: Add a null pointer check

… > This patch will add the malloc failure checking … * Please use a corresponding imperative wording for the change description. * Would you like to add the tag “Fixes” accordingly? Regards, Markus

Re: [PATCH v4 1/2] kunit: unregister the device on error

> > kunit_init_device() should unregister the device on bus register error, > > but mistakenly it tries to unregister the bus. > > > > Unregister the device instead of the bus. … > Reviewed-by: Greg Kroah-Hartman Would you ever like to distinguish hardware register errors from item registration f

Re: [PATCH v4 0/2] kunit: fix minor error path mistakes

… > * Remove some changes requested by Marcus Elfring, I became curious how affected software components can evolve further. > as I was alerted he is a known troll. I would appreciate if this interpretation will be reconsidered somehow. Regards, Markus

Re: [PATCH v3 2/2] kunit: avoid memory leak on device register error

> If the device register fails, free the allocated memory before > returning. Can a description variant (like the following) be more appropriate? Free the allocated memory (after a device registration failure) before returning. Thus add a jump target so that a bit of exception handling c

Re: [PATCH v3 1/2] kunit: unregister the device on error

> kunit_init_device() should unregister the device on bus register error, > but mistakenly it tries to unregister the bus. Would the following description variant be more appropriate? kunit_init_device() should unregister the device on bus registration error. But it mistakenly tries to unre

Re: [ 2/2] kunit: avoid memory leak on device register error

>> Common error handling code can be used instead >> if an additional label would be applied for a corresponding jump target. >> >> How do you think about to increase the application of scope-based resource >> management here? > > I thought about that. But I think the code is simple enough (for no

Re: [PATCH 2/2] kunit: avoid memory leak on device register error

> If the device register fails, free the allocated memory before > returning. * I suggest to use the word “registration” (instead of “register”) in the commit message. * Would you like to add the tag “Fixes” accordingly? > +++ b/lib/kunit/device.c > @@ -131,6 +131,7 @@ static struct kunit_dev

Re: [PATCH 1/2] kunit: unregister the device on error

> kunit_init_device() should unregister the device on bus register error. * Would another imperative wording be desirable also for this change description? * Will the tag “Fixes” become relevant here? Regards, Markus