Re: [PATCH] remoteproc: mediatek: Add SCP watchdog handler in IRQ processing

> In mt8195_scp_c1_irq_handler(), only the IPC interrupt bit > (MT8192_SCP_IPC_INT_BIT) was checked., but does not handle > when this bit is not set. This could lead to unhandled watchdog > events. This could lead to unhandled watchdog events. A proper … * You may occasionally put more than 63 cha

Re: [PATCH bpf-next 1/2] bpf: Create cgroup storage if needed when updating link

… > This cause a painc when accessing stroage in bpf_get_local_storage. Please avoid typos in such a change description. How do you think about to append parentheses to function names? See also: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/su

Re: [PATCH v3] media: add virtio-media driver

… > +++ b/drivers/media/virtio/virtio_media_driver.c > @@ -0,0 +1,959 @@ … > +static struct virtio_media_session * > +virtio_media_session_alloc(struct virtio_media *vv, u32 id, > +bool nonblocking_dequeue) > +{ … > + init_waitqueue_head(&session->dqbuf_wait); > + > +

Re: [PATCH v8 3/8] vhost: Add the cgroup related function

… > +++ b/drivers/vhost/vhost.c … > +static int vhost_attach_task_to_cgroups(struct vhost_worker *worker) > +{ … > + vhost_worker_queue(worker, &attach.work); > + > + mutex_lock(&worker->mutex); … > + mutex_unlock(&worker->mutex); > + > + return attach.ret; > +} … Under which circu

Re: [PATCH v2] openrisc: Add cacheinfo support

… > This patch provides a mechanism … > The patch also moves … See also: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc7#n94 Regards, Markus

Re: [PATCH v2] selftests/mm/cow: Fix the incorrect error handling

… > This patch will fix it. Will an imperative wording be more desirable for such a change description? https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc6#n94 Regards, Markus

Re: [Patch net 2/2] vsock/virtio: Don't reset the created SOCKET during suspend to ram

… > and it will cause that socket can not be unusable after resume. … I find such a wording confusing. Can the change description become clearer? Regards, Markus

Re: [PATCH v2] virtio: fix reference leak in register_virtio_device()

> Once device_add(&dev->dev) failed, call put_device() to explicitly > release dev->dev. Or it could cause double free problem. https://elixir.bootlin.com/linux/v6.13-rc3/source/drivers/base/core.c#L3521 … > Found by code review. https://elixir.bootlin.com/linux/v6.13-rc3/source/drivers/virtio/

Re: vhost-vdpa: Refactor copy_to_user() usage in vhost_vdpa_get_config()

>> Assign the return value from a copy_to_user() call to an additional >> local variable so that a kvfree() call and return statement can be >> omitted accordingly. > > Ugly and unidiomatic. > >> This issue was detected by using the Coccinelle software. > > What issue? Opportunities for the reduct

[PATCH] vhost-vdpa: Refactor copy_to_user() usage in vhost_vdpa_get_config()

From: Markus Elfring Date: Wed, 25 Sep 2024 20:36:35 +0200 Assign the return value from a copy_to_user() call to an additional local variable so that a kvfree() call and return statement can be omitted accordingly. This issue was detected by using the Coccinelle software. Signed-off-by: Markus

[PATCH] remoteproc: qcom: q6v5-mss: Use common error handling code in q6v5_mpss_load()

From: Markus Elfring Date: Tue, 24 Sep 2024 15:55:06 +0200 Add jump targets so that a bit of exception handling can be better reused at the end of this function implementation. Signed-off-by: Markus Elfring --- drivers/remoteproc/qcom_q6v5_mss.c | 17 + 1 file changed, 9

[PATCH] remoteproc: k3: Call of_node_put(rmem_np) only once in three functions

From: Markus Elfring Date: Tue, 24 Sep 2024 14:28:35 +0200 An of_node_put(rmem_np) call was immediately used after a pointer check for a of_reserved_mem_lookup() call in three function implementations. Thus call such a function only once instead directly before the checks. This issue was

[PATCH] nvdimm: Call nvdimm_put_key(key) only once in nvdimm_key_revalidate()

From: Markus Elfring Date: Mon, 23 Sep 2024 17:05:39 +0200 A nvdimm_put_key(key) call was immediately used after a return code check for a change_key() call in this function implementation. Thus call such a function only once instead directly before the check. This issue was transformed by

Re: [PATCH] driver core: Fix a null-ptr-deref in module_add_driver()

> Inject fault while probing of-fpga-region, if kasprintf() fails in > module_add_driver(), the second sysfs_remove_link() in exit path will cause > null-ptr-deref as below because kernfs_name_hash() will call strlen() with > NULL driver_name. … How do you think about to use the term “null pointer

[PATCH] tracing: Replace 21 seq_puts() calls by seq_putc() calls

From: Markus Elfring Date: Sun, 14 Jul 2024 15:40:34 +0200 Single characters should be put into a sequence. Thus use the corresponding function “seq_putc”. This issue was transformed by using the Coccinelle software. Signed-off-by: Markus Elfring --- kernel/trace/trace_events_hist.c | 26

[PATCH] module: tracking: Extend format string of a seq_printf() call in unloaded_tainted_modules_seq_show()

From: Markus Elfring Date: Sun, 14 Jul 2024 13:43:06 +0200 Subject: [PATCH] module: tracking: Extend format string of a seq_printf() call in unloaded_tainted_modules_seq_show() * Move the specification of a single line break from a seq_puts() call into the format string of a previous

[PATCH] module: Use seq_putc() in two functions

From: Markus Elfring Date: Sun, 14 Jul 2024 13:13:15 +0200 Single characters should be put into a sequence. Thus use the corresponding function “seq_putc”. This issue was transformed by using the Coccinelle software. Signed-off-by: Markus Elfring --- kernel/module/procfs.c | 4 ++-- 1 file

Re: [PATCH] remoteproc: Remove unneeded check in elf_strtbl_add()

… > useless. because …? > Fix this issue by removing unneeded check. Another wording suggestion: Thus remove a redundant check. … > +++ b/drivers/remoteproc/remoteproc_elf_helpers.h > @@ -107,7 +107,7 @@ static inline unsigned int elf_strtbl_add(const char > *name, void *ehdr, u8

Re: [v4 2/3] leds: sy7802: Add support for Silergy SY7802 flash LED controller

>> Various source code places can be updated also according to referenced >> programming interfaces. >> https://elixir.bootlin.com/linux/v6.10-rc4/source/include/linux/cleanup.h#L8 >> >> Will corresponding collateral evolution become better supported? > > Plesae stop this. cleanup.h might be a nice

Re: [v4 2/3] leds: sy7802: Add support for Silergy SY7802 flash LED controller

>> Would you become interested to apply a statement like >> “guard(mutex)(&chip->mutex);”? >> https://elixir.bootlin.com/linux/v6.10-rc3/source/include/linux/mutex.h#L196 > > This does not look like real improvement for code this trivial. Various source code places can be updated also according t

Re: [PATCH v4 2/3] leds: sy7802: Add support for Silergy SY7802 flash LED controller

> The SY7802 is a current-regulated charge pump which can regulate two > current levels for Flash and Torch modes. > > It is a high-current synchronous boost converter with 2-channel high > side current sources. Each channel is able to deliver 900mA current. Would you like to improve such a change

Re: [PATCH] livepatch: introduce klp_func called interface

Please add a version identifier to the message subject. … > If the patched function have bug, it may cause serious result > such as kernel crash. Wording suggestion: If the patched function has a bug, it might cause serious side effects like a kernel crash. > This is a kobject attribute

Re: [PATCH] livepatch: introduce klp_func called interface

… > This commit introduce a read only interface of livepatch Please improve the changelog with an imperative wording. … > find out which function is successfully called. Any testing process can make > sure they > have successfully cover all the patched function that changed with the help > of

Re: [PATCH] eventfs: Directly return NULL to avoid null point dereferenced

> When the condition ei->is_free holds,we return NULL directly to > avoid update_events_attr to use NULL point about ei. * Please avoid typos in the summary phrase and the commit message. * Would you like to use an imperative wording for an improved change description? https://git.kernel.org/

Re: [v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

… > > it jumps to the label 'out' instead of 'fail' by mistake.In the result, … > > Looks good to me. * Do you care for a typo in this change description? * Would you like to read any improved (patch) version descriptions (or changelogs)? Regards, Markus

Re: [PATCH v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

> … by mistake.In the result, … I propose once more to start the second sentence of this change description on a subsequent line. > --- > kernel/trace/trace_probe.c | 2 +- … Unfortunately, you overlooked to add patch version descriptions behind the ma

Re: [PATCH v2] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body

I suggest to append parentheses to the function name in the summary phrase. > If traceprobe_parse_probe_arg_body() fails to allocate 'parg->fmt', it > jumps to 'out' instead of 'fail' by mistake. In the result, in this > case the 'tmp' buffer is not freed and leaks its memory. > > Fix it by jumpi

Re: tracing/probes: Fix a memory leak in traceprobe_parse_probe_arg_body()

… > > If traceprobe_parse_probe_arg_body() fails to allocate 'parg->fmt', it > jumps to 'out' instead of 'fail' by mistake. In the result, in this > case the 'tmp' buffer is not freed and leaks its memory. > > Fix it by jumping to 'fail' in that case. > … How do you think about anoth

Re: [PATCH] tracing/probes: Fix memory leak issues in traceprobe_parse_probe_arg_body()

… > Therefore, the program should jump to the fail label instead of the out > label. This commit fixes this bug. > > Signed-off-by: LuMingYin <11570291+yin-lum...@user.noreply.gitee.com> Please improve your patch attempt considerably. See also: https://kernelnewbies.org/FirstKernelPatch Are th

Re: [PATCH v2] ftrace: Fix possible use-after-free issue in ftrace_location()

… > To fix it, we hold rcu lock as lookuping ftrace record, and call > synchronize_rcu() before freeing any ftrace pages. I suggest to convert this description into an imperative wording. Regards, Markus

Re: [v2] ice: Fix freeing uninitialized pointers

>>> Automatically cleaned up pointers need to be initialized before exiting >>> their scope. In this case, they need to be initialized to NULL before >>> any return statement. >> >> * May we expect that compilers should report that affected variables >> were only declared here instead of appropr

Re: [PATCH v2 net] ice: Fix freeing uninitialized pointers

> Automatically cleaned up pointers need to be initialized before exiting > their scope. In this case, they need to be initialized to NULL before > any return statement. * May we expect that compilers should report that affected variables were only declared here instead of appropriately defined

Re: [2/2] virtiofs: Improve error handling in virtio_fs_get_tree()

>> It is probably clear that the function call “kfree(NULL)” does not perform >> data processing which is really useful for the caller. >> Such a call is kept in some cases because programmers did not like to invest >> development resources for its avoidance. > > on the contrary, it is extremely us

Re: [2/2] virtiofs: Improve error handling in virtio_fs_get_tree()

> Do you consider more clarity in your argumentation? It is probably clear that the function call “kfree(NULL)” does not perform data processing which is really useful for the caller. Such a call is kept in some cases because programmers did not like to invest development resources for its avoidan

Re: [2/2] virtiofs: Improve error handling in virtio_fs_get_tree()

>>> So what? kfree(NULL) is perfectly acceptable. >> >> I suggest to reconsider the usefulness of such a special function call. > > Can you be more explicit in your suggestion? I hope that the change acceptance can grow for the presented transformation. Are you looking for an improved patch descr

Re: [PATCH 2/2] virtiofs: Improve error handling in virtio_fs_get_tree()

>> The kfree() function was called in two cases by >> the virtio_fs_get_tree() function during error handling >> even if the passed variable contained a null pointer. > > So what? kfree(NULL) is perfectly acceptable. I suggest to reconsider the usefulness of such a special function call. > Are

[PATCH 2/2] virtiofs: Improve error handling in virtio_fs_get_tree()

From: Markus Elfring Date: Fri, 29 Dec 2023 09:15:07 +0100 The kfree() function was called in two cases by the virtio_fs_get_tree() function during error handling even if the passed variable contained a null pointer. This issue was detected by using the Coccinelle software. * Thus use another

[PATCH 1/2] virtiofs: Improve three size determinations

From: Markus Elfring Date: Fri, 29 Dec 2023 08:42:04 +0100 Replace the specification of data structures by pointer dereferences as the parameter for the operator “sizeof” to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was

[PATCH 0/2] virtiofs: Adjustments for two function implementations

From: Markus Elfring Date: Fri, 29 Dec 2023 09:28:09 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): Improve three size determinations Improve error handling in virtio_fs_get_tree() fs/fuse/virtio_fs.c | 19

Re: [v5 0/1] dax: enable dax fault handler to report VM_FAULT_HWPOISON

>> Would you insist on the usage of cover letters also for single patches? > > I would neither insist on it, nor prohibit it. It seems that you can tolerate an extra message here. > It simply does not make enough difference. Can it occasionally be a bit nicer to receive all relevant information

Re: [v5 0/1] dax: enable dax fault handler to report VM_FAULT_HWPOISON

>> How do you think about to put additional information below triple dashes >> (or even into improved change descriptions)? >> >> See also: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n686 > > Markus, > > Please go a

Re: [v5 0/1] dax: enable dax fault handler to report VM_FAULT_HWPOISON

> The thought was to put descriptions unsuitable for commit header in the cover > letter. How do you think about to put additional information below triple dashes (or even into improved change descriptions)? See also: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume

Re: [PATCH v5 0/1] dax: enable dax fault handler to report VM_FAULT_HWPOISON

> Change from v4: … I suggest to omit the cover letter for a single patch. Will any patch series evolve for your proposed changes? Regards, Markus

Re: nvdimm: Replace the usage of a variable by a direct function call in nd_pfn_validate()

> FYI - I'm a tiny bit taken aback that in response to me applying, > and providing feedback, on your patch, This will probably trigger collateral evolution, won't it? > you respond with 2 links for me to follow I offered another bit of background information according to your enquiry. > and

Re: [PATCH] nvdimm: Replace the usage of a variable by a direct function call in nd_pfn_validate()

>> The address of a data structure member was determined before >> a corresponding null pointer check in the implementation of >> the function “nd_pfn_validate”. >> >> Thus avoid the risk for undefined behaviour by replacing the usage of >> the local variable “parent_uuid” by a direct function call

[PATCH] nvdimm: Replace the usage of a variable by a direct function call in nd_pfn_validate()

a direct function call within a later condition check. This issue was detected by using the Coccinelle software. Fixes: d1c6e08e7503649e4a4f3f9e700e2c05300b6379 ("libnvdimm/labels: Add uuid helpers") Signed-off-by: Markus Elfring --- drivers/nvdimm/pfn_devs.c | 3 +-- 1 file

Re: [PATCH] net/mlx5e: fix bpf_prog reference count leaks in mlx5e_alloc_rq

… > The refcount leak issues take place in two error handling paths. Can an other wording be a bit nicer for the commit message? > Fix the issue by … I suggest to replace this wording by the tag “Fixes”. Regards, Markus

Re: [PATCH V2] dmaengine: bcm-sba-raid: add missing put_device() call in sba_probe()

> if of_find_device_by_node() succeed, sba_probe() doesn't have a > corresponding put_device(). … Wording adjustment: If a of_find_device_by_node() call succeeded, sba_probe() did not contain a corresponding put_device() call. … Regards, Markus

Re: [PATCH] atm: Fix atm_dev reference count leaks in atmtcp_remove_persistent()

… > The refcount leaks issues occur in two error handling paths. Can it be nicer to use the term “reference count” for the commit message? > Fix the issue by … I suggest to replace this wording by the tag “Fixes”. … > +++ b/drivers/atm/atmtcp.c > @@ -433,9 +433,15 @@ static int atmtcp_remove

Re: [PATCH] ethernet: fix potential memory leak in gemini_ethernet_port_probe()

> If some processes in gemini_ethernet_port_probe() fail, > free_netdev(dev) needs to be called to avoid a memory leak. Would you like to use an imperative wording for this change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-

Re: [PATCH] net: nixge: fix potential memory leak in nixge_probe()

> If some processes in nixge_probe() fail, free_netdev(dev) > needs to be called to aviod a memory leak. * Would you like to avoid a typo in this change description? * An imperative wording can be preferred here, can't it? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree

Re: [PATCH] ARM: zx: remove redundant dev_err call in zx296702_pd_probe()

… > +++ b/arch/arm/mach-zx/zx296702-pm-domain.c > @@ -170,7 +170,6 @@ static int zx296702_pd_probe(struct platform_device *pdev) > > pcubase = devm_ioremap_resource(&pdev->dev, res); > if (IS_ERR(pcubase)) { > - dev_err(&pdev->dev, "ioremap fail.\n"); > return

Re: [PATCH] m68k/amiga: Add missing platform_device_unregister() call in amiga_init_devices()

> Add the missing platform_device_unregister() before return > from amiga_init_devices() in the error handling case. Will the tag “Fixes” become helpful for the commit message? … > +++ b/arch/m68k/amiga/platform.c > @@ -188,8 +188,10 @@ static int __init amiga_init_devices(void) >

Re: [PATCH] usb: hso: check for return value in hso_serial_common_create()

> in case of an error tty_register_device_attr() returns ERR_PTR(), > add IS_ERR() check I suggest to improve this change description a bit. Will the tag “Fixes” become helpful for the commit message? … > +++ b/drivers/net/usb/hso.c … > @@ -2311,6 +2313,7 @@ static int hso_serial_common_create

Re: [PATCH v3 0/3] Update memdup_user.cocci

… > > Changes in v3: > > - add missing '-' for patch rule in kmalloc/kzalloc call args > > - selfcheck rule dropped from patchset … > All three applied. … Will the software development discussion be continued by patches according to previously mentioned ideas and remaining open issues? Regards,

Re: [Cocci] [PATCH v3 0/3] Update memdup_user.cocci

> All three applied. … Can the accepted software adjustment be seen by the interface of a public development repository? Regards, Markus

Re: [PATCH v2] ARM: milbeaut: Add missing of_node_put() call in m10v_smp_init()

> The variable np in function m10v_smp_init takes the return value > of of_find_compatible_node, which gets a node but does not put it. Such information is useful. > If this node is not put it may cause a memory leak. Is the reference management generally improvable for this function implement

Re: [PATCH] block: Fix reference count leak in blk_integrity_add

> kobject_init_and_add() takes reference even when it fails. If this > function returns an error, kobject_put() must be called to properly > clean up the memory associated with the object. * An imperative wording can be preferred for the change description, can't it? * Would you like to add the

Re: [PATCH] sh: sh4: Fix reference count leak in sq_dev_add

> kobject_init_and_add() takes reference even when it fails. If this > function returns an error, kobject_put() must be called to properly > clean up the memory associated with the object. * An imperative wording can be preferred for the change description, can't it? * Would you like to add the

Re: [PATCH] sh: sh4: Fix reference count leak in sq_dev_add

> kobject_init_and_add() takes reference even when it fails. If this > function returns an error, kobject_put() must be called to properly > clean up the memory associated with the object. * An imperative wording can be preferred for the change description, can't it? * Would you like to add the

Re: [PATCH] nbd: add missed destroy_workqueue when nbd_start_device fails

> destroy_workqueue() should be called to destroy ndev->tx_wq > when nbd_start_device init resources fails. * An imperative wording can be preferred for the change description, can't it? * Would you like to add the tag “Fixes” to the commit message? Regards, Markus

Re: [PATCH] RDMA/core: Fix return error value to negative in _ib_modify_qp()

Please add a change description (according to a missing minus character). Regards, Markus

Re: [PATCH] ipv6: Fix nexthop reference count in ip6_route_info_create()

… > When ip6_route_info_create() returns, local variable "nh" becomes > invalid, so the refcount should be decreased to keep refcount balanced. Can it be helpful to use the term “reference count” in consistent ways? > Fix this issue How do you think about to replace this wording by the tag “Fix

Re: [PATCH] regulator: fix memory leak on error path of regulator_register()

> … introduced as a side ef another … Would the following wording variant be more appropriate? … introduced as a side effect of another … How do you think about to replace the wording “…, I believe …” by an imperative description? Regards, Markus

Re: [f2fs-dev] [PATCH v2] f2fs: fix use-after-free issue in f2fs_put_super()

> During umount, … Do you refer to the action “unmount” here? > f2fs_destroy_segment_manager(), it may cause … Wording adjustments: f2fs_destroy_segment_manager(). It might cause … > … with procfs accessing, … Avoid another typo?: … with procfs accesses, … > …, fix it by … Please replace

Re: [PATCH] media: davinci: vpif_capture: fix potential double free

> In case of errors vpif_probe_complete() releases memory for vpif_obj.sd > and unregisters the V4L2 device. But then this is done again by > vpif_probe() itself. The patch removes the cleaning from > vpif_probe_complete(). * An imperative wording can be preferred for the change description, can

Re: [PATCH] media: camss: fix memory leaks on error handling paths in probe

> camss_probe() does not free camss on error handling paths. The patch > introduces an additional error label for this purpose. * I suggest to use an imperative wording for the change description. * Would you like to use also a jump target like the following at the end of this function implemen

Re: [PATCH] ARM: milbeaut: Add missing of_node_put()

> After finishing using device node got from of_find_compatible_node(), > of_node_put() needs to be called. * An imperative wording can be preferred for the change description, can't it? * Will the tag “Fixes” become helpful for the commit message? Regards, Markus

Re: [PATCH] efi: add missed destroy_workqueue when efisubsys_init fails

> destroy_workqueue() should be called to destroy efi_rts_wq > when efisubsys_init() init resources fails. * Can such exception handling depend on the data structure member “efi.runtime_supported_mask”? * An imperative wording would be preferred for the change description (besides another bit

Re: [PATCH] mailbox: pcc: Put the PCCT table for error path

… > In acpi_pcc_probe(), the PCCT table entries will be used as private > data for communication chan at runtime, but the table should be put > for error path. * An imperative wording can be preferred for the change description, can't it? * Will the tag “Fixes” become helpful for the commit mes

Re: [PATCH] drm/amdkfd: Put ACPI table after using it

… > and in kfd_create_vcrat_image_cpu(), the acpi_table is only used to > get the OEM info, so those table mappings need to be release after … 1. Please avoid a typo for this change description. 2. An imperative wording can be preferred here, can't it? 3. Will the tag “Fixes” become helpful for

Re: [v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()

>> Under which circumstances would you begin to care more for involved >> differences >> in corresponding run time characteristics? > > When the difference is hours vs seconds. Such a view can be influenced by execution environments in considerable ways. > In this case, there are tradeoffs betw

Re: [v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()

> Markus, you are welcome to try this since you are concerned about it. I dare to point software design variations for some reasons. > But it doesn't matter. Under which circumstances would you begin to care more for involved differences in corresponding run time characteristics? Regards, Mark

Re: [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()

>>> +@depends on patch@ >>> +expression from,to,size; >>> +identifier l1,l2; >>> +@@ >>> + >>> +- to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); >>> ++ to = vmemdup_user(from,size); >> >> I propose to combine the desired adjustment with the previous SmPL rule >> by using another disj

Re: [v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()

> This is fine as is in all three aspects. I have tried again to point specific data processing consequences out for operation modes of scripts in the semantic patch language. Regards, Markus

Re: [PATCH v3 3/3] coccinelle: api: filter out memdup_user definitions

> Don't match memdup_user/vmemdup_user. I find such a change description insufficient. Regards, Markus

Re: [PATCH v3 2/3] coccinelle: api: extend memdup_user rule with vmemdup_user()

… > +++ b/scripts/coccinelle/api/memdup_user.cocci > @@ -39,6 +39,28 @@ … … > +@depends on patch@ > +expression from,to,size; > +identifier l1,l2; > +@@ > + > +- to = \(kvmalloc\|kvzalloc\)(size,\(GFP_KERNEL\|GFP_USER\)); > ++ to = vmemdup_user(from,size); I propose to combine the desired adjust

Re: [PATCH v3 1/3] coccinelle: api: extend memdup_user transformation with GFP_USER

> Match GFP_USER and optional __GFP_NOWARN allocations with > memdup_user.cocci rule. I suggest to clarify software design consequences according to such information a bit more. I find it helpful if you would have included also my email address directly in the message field “To” or “Cc”. Are the

Re: [PATCH v2] drm/virtio: Fix memory leak in virtio_gpu_execbuffer_ioctl()

> … To balance the reference > count initialized when allocating fence, dma_fence_put() > should not be deleted. * Would an imperative wording be more appropriate for the change description? * Is the information “hexin” sufficient for a real name? Regards, Markus

Re: [PATCH] clk: ti: clkctrl: add the missed kfree() for _ti_omap4_clkctrl_setup()

… > +++ b/drivers/clk/ti/clkctrl.c > @@ -655,8 +655,10 @@ static void __init _ti_omap4_clkctrl_setup(struct > device_node *node) > } > > hw = kzalloc(sizeof(*hw), GFP_KERNEL); > - if (!hw) > + if (!hw) { > + kfree(clkctrl_name

Re: [PATCH] clk: ti: clkctrl: add the missed kfree() for _ti_omap4_clkctrl_setup()

… > +++ b/drivers/clk/ti/clkctrl.c > @@ -655,8 +655,10 @@ static void __init _ti_omap4_clkctrl_setup(struct > device_node *node) > } > > hw = kzalloc(sizeof(*hw), GFP_KERNEL); > - if (!hw) > + if (!hw) { > + kfree(clkctrl_name

Re: [f2fs-dev] [PATCH] f2fs: compress: Avoid memory leak on cc->cpages

>>> Memory allocated for storing compressed pages' poitner should be >>> released after f2fs_write_compressed_pages(), otherwise it will >>> cause memory leak issue. >> >> * Would an imperative wording be more appropriate (without a typo) >>   for the change description? >> >> * Will the tag “Fixes

Re: [PATCH] ipmi: Remove duplicate code in __ipmi_bmc_register()

> > __ipmi_bmc_register() jumps to the label 'out_free_my_dev_name' in an > > error path. So we can remove duplicate code in the if (rv). > > Looks correct, queued for next release. 1. Can an imperative wording be preferred for the change description? 2. Will the tag “Fixes” become helpful for th

Re: [PATCH] drm/virtio: Fix memory leak in virtio_gpu_execbuffer_ioctl()

* I suggest to add a change description. * Is “hexin.op” a real name? Regards, Markus

Re: [f2fs-dev] [PATCH] f2fs: compress: Avoid memory leak on cc->cpages

> Memory allocated for storing compressed pages' poitner should be > released after f2fs_write_compressed_pages(), otherwise it will > cause memory leak issue. * Would an imperative wording be more appropriate (without a typo) for the change description? * Will the tag “Fixes” become helpful fo

Re: [PATCH] ath11k: Fix memory leak in ath11k_qmi_init_service()

> When qmi_add_lookup fail, we should destroy the workqueue Can the following wording variant be nicer for the change description? Destroy the work queue object in an if branch after a call of the function “qmi_add_lookup” failed. Regards, Markus

Re: [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

>> you do indeed need to put - in front of the second and third lines as well. > > Thanks, Markus, Julia. I will send v3. How do you think about to discuss any remaining open issues already on the current software development version for your transformation approach? Regards, Markus

Re: [PATCH] mt76: mt76u: add missing release on skb in __mt76x02u_mcu_send_msg

… > +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c … > @@ -111,6 +113,7 @@ __mt76x02u_mcu_send_msg(struct mt76_dev *dev, struct > sk_buff *skb, > if (wait_resp) > ret = mt76x02u_mcu_wait_resp(dev, seq); > > +out: > consume_skb(skb); … I suggest to use the lab

Re: [PATCH] mt7601u: add missing release on skb in mt7601u_mcu_msg_send

… > +++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c > @@ -116,8 +116,10 @@ mt7601u_mcu_msg_send(struct mt7601u_dev *dev, struct > sk_buff *skb, > int sent, ret; > u8 seq = 0; > > - if (test_bit(MT7601U_STATE_REMOVED, &dev->state)) > + if (test_bit(MT7601U_STATE_REMOVED, &dev-

Re: [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

> You proposed essentially \( A \| B \) \( | C \| \) Will the software development attention grow for a topic like “Extend support for handling of optional source code parts”? https://github.com/coccinelle/coccinelle/issues/53 How do you think about another tiny example script for the semantic p

Re: [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

* https://lore.kernel.org/cocci/5c0dae88-e172-3ba6-f86c-d1a6238bb...@web.de/ https://lkml.org/lkml/2020/6/9/568 >>> >>> This one it complete nonsense. >> >> I hope that different views can be clarified for such a software situation >> in more constructive ways. > > You proposed es

Re: [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

>>> Applied. >> >> Do you care for patch review concerns according to this SmPL script >> adjustment? >> >> * https://lore.kernel.org/cocci/5c0dae88-e172-3ba6-f86c-d1a6238bb...@web.de/ >> https://lkml.org/lkml/2020/6/9/568 > > This one it complete nonsense. I hope that different views can be cl

Re: [v2 1/4] coccinelle: api: extend memdup_user transformation with GFP_USER

> Applied. Do you care for patch review concerns according to this SmPL script adjustment? * https://lore.kernel.org/cocci/5c0dae88-e172-3ba6-f86c-d1a6238bb...@web.de/ https://lkml.org/lkml/2020/6/9/568 * https://lore.kernel.org/cocci/c3464cad-e567-9ef5-b4e3-a01e3b111...@web.de/ https://lkml

Re: [PATCH] net: cxgb3: add missed destroy_workqueue in cxgb3 probe failure

> Add the missed calls to fix it. You propose to add only a single function call. … > +++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c > @@ -3407,6 +3407,7 @@ static int init_one(struct pci_dev *pdev, const struct > pci_device_id *ent) > out_disable_device: > pci_disable_device(pdev

Re: [PATCH v4 1/2] drivers: provide devm_platform_request_irq()

> v3 -> v4: > - The patch v3 sent on May 27 may be lost somewhere in the > world, so resend it. Can previous patch review aspects get any more attention? https://lore.kernel.org/cocci/5dad9b19-ceb5-1606-9f62-7626e5677...@web.de/ https://lkml.org/lkml/2020/5/27/1326 Regards, Markus

Re: [PATCH v4] coccinelle: api: add kzfree script

I dare to repeat previous patch review aspects once more. https://lore.kernel.org/cocci/a316f076-1686-25d8-18fe-1bbc0cf9a...@web.de/ … > +virtual context > +virtual patch > +virtual org > +virtual report +virtual context, patch, org, report Is such a SmPL code variant more succinct? … > +if (.

Re: [PATCH v2] nfc: nci: add missed destroy_workqueue in nci_register_device

> When nfc_register_device fails in nci_register_device, > destroy_workqueue() shouled be called to destroy ndev->tx_wq. Would an other imperative wording be preferred for the commit message? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-p

Re: [PATCH v4 1/8] irqchip/loongson-htpic: Remove redundant kfree operation

> In the function htpic_of_init(), when kzalloc htpic fails, it should > return -ENOMEM directly, no need to execute "goto" to kfree. Would another imperative wording be preferred for the commit message? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/

Re: [PATCH] net: smc91x: Fix possible memory leak in smc_drv_probe()

> If try_toggle_control_gpio() failed in smc_drv_probe(), free_netdev(ndev) > should be called to free the ndev created earlier. Otherwise, a memleak > will occur. * Will it be nicer to use the term “memory leak” also in this change description? * Would another imperative wording be preferred fo

Re: [PATCH v2] tipc: Don't using smp_processor_id() in preemptible code

* I find the patch subject improvable. * Please add a change description for the desired commit. Regards, Markus

  1   2   3   4   5   6   7   8   9   10   >