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

2020-07-29 Thread Markus Elfring
… > 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] atm: Fix atm_dev reference count leaks in atmtcp_remove_persistent()

2020-07-29 Thread Markus Elfring
… > 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()

2020-07-29 Thread Markus Elfring
> 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()

2020-07-29 Thread Markus Elfring
> 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] usb: hso: check for return value in hso_serial_common_create()

2020-07-28 Thread Markus Elfring
> 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] ipv6: Fix nexthop reference count in ip6_route_info_create()

2020-07-25 Thread Markus Elfring
… > 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] ath11k: Fix memory leak in ath11k_qmi_init_service()

2020-07-20 Thread Markus Elfring
> 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: [PATCH] mt76: mt76u: add missing release on skb in __mt76x02u_mcu_send_msg

2020-07-18 Thread Markus Elfring
… > +++ 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

2020-07-18 Thread Markus Elfring
… > +++ 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: [PATCH] net: cxgb3: add missed destroy_workqueue in cxgb3 probe failure

2020-07-17 Thread Markus Elfring
> 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 v2] nfc: nci: add missed destroy_workqueue in nci_register_device

2020-07-17 Thread Markus Elfring
> 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] net: smc91x: Fix possible memory leak in smc_drv_probe()

2020-07-15 Thread Markus Elfring
> 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

2020-07-15 Thread Markus Elfring
* I find the patch subject improvable. * Please add a change description for the desired commit. Regards, Markus

Re: [PATCH] net: xilinx: fix potential NULL dereference in temac_probe()

2020-07-14 Thread Markus Elfring
> If you use devm_ioremap_resource() you can remove the !res check > entirely which would be equally acceptable as a fix. Would you like to use the wrapper function “devm_platform_get_and_ioremap_resource” then? https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/base/platform.c#L66 https://

Re: [PATCH v3] net: phy: smsc: fix printing too many logs

2020-06-21 Thread Markus Elfring
> out and this commit was modified to prints an error message. it is an > inappropriate conversion by used phy_read_poll_timeout() to introduce > this bug. so fix this issue by use read_poll_timeout() to replace > phy_read_poll_timeout(). Will another wording variant be nicer (with less typos) for

Re: [PATCH] macvlan: Fix memory leak in macvlan_changelink_sources()

2020-06-18 Thread Markus Elfring
> -->If fail, need to free previous malloc memory I suggest to improve this change description. How does the proposed addition of the function call “macvlan_flush_sources” fit to this information? Regards, Markus

Re: [PATCH v3 2/2] perf tools: Fix potential memory leaks in perf events parser

2020-06-15 Thread Markus Elfring
> Fix potential memory leak. Function new_term may return error, so > it is need to free memory when the return value is negative. I hope that a typo will be avoided for the final commit message. Would you find any other description variant more pleasing? Regards, Markus

Re: [PATCH v3 1/2] perf tools: Fix potential memory leaks in perf events parser

2020-06-15 Thread Markus Elfring
> Fix memory leak of in function parse_events_term__sym_hw() > and parse_events_term__clone() when string duplication failed. Can a wording like “Fix memory leaks in …” be more appropriate for the final commit message? Would you find any other description variant more pleasing? Regards, Markus

Re: [PATCH v3 0/2] Fixing memory leaks in perf events parser

2020-06-15 Thread Markus Elfring
> fix some memleaks in parse_events_term__sym_hw and parse_events_term__clone. Can it be more appropriate to refer to the term “memory leak” in consistent ways? > v1 ==> v2 > 1. split into two patches Corresponding development consequences can become more interesting. > v2 ==> v3 > add more

Re: [PATCH] net: macb: fix ref count leaking via pm_runtime_get_sync

2020-06-14 Thread Markus Elfring
> in macb_mdio_write, … * Will a desire evolve to improve also this commit message? * Will the tag “Fixes” become helpful? … > +++ b/drivers/net/ethernet/cadence/macb_main.c … > @@ -3840,11 +3842,14 @@ static int at91ether_open(struct net_device *dev) > > ret = macb_phylink_connect(lp);

Re: [PATCH] net: fec: fix ref count leaking when pm_runtime_get_sync fails

2020-06-14 Thread Markus Elfring
> in fec_enet_mdio_read, … I am curious under which circumstances you would like to improve such commit messages. * Will the tag “Fixes” become helpful? * Which source code analysis tools did trigger to send update suggestions according to 16 similar issues for today? … > +++ b/drivers/net/e

Re: [PATCH] test_objagg: Fix memory leak in test_hints_case()

2020-06-12 Thread Markus Elfring
> … The patch fixes this issue. I propose to replace this information by the tag “Fixes”. Please choose another imperative wording for your change description. Regards, Markus

Re: [PATCH] ethernet: Fix memory leak in ethoc_probe()

2020-06-12 Thread Markus Elfring
> … The patch fixes this issue. I propose to replace this information by the tag “Fixes”. Please choose another imperative wording for your change description. Regards, Markus

Re: [PATCH] rocker: Fix error handling in dma_rings_init()

2020-06-12 Thread Markus Elfring
> … The patch fixes the > order consistent with cleanup in rocker_dma_rings_fini(). I suggest to choose another imperative wording for your change description. Will the tag “Fixes” become helpful for the commit message? Regards, Markus

Re: [PATCH] NFC: Fix error handling in rawsock_connect()

2020-06-12 Thread Markus Elfring
> … The patch fixes this issue. I suggest to replace this information by the tag “Fixes”. Please choose another imperative wording for your change description. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=df2fbf5bfa0e7fff8b

Re: [PATCH v2 2/2] perf tools: Improve exception handling in two functions of perf events parser

2020-06-11 Thread Markus Elfring
> Fix potential memory leak. Function new_term may return error, so > it is need to free memory when the return value is negative. How do you think about a wording variant like the following? Add jump targets so that a configuration object and a duplicated string are released after a call o

Re: [PATCH v2 1/2] perf tools: Fix potential memory leaks in perf events parser

2020-06-11 Thread Markus Elfring
> Fix memory leak of in function parse_events_term__sym_hw() > and parse_events_term__clone() when error occur. How do you think about a wording variant like the following? Release a configuration object after a string duplication failed. Regards, Markus

Re: [PATCH] perf tools: Fix potential memory leak in perf events parser

2020-06-11 Thread Markus Elfring
> Fix potential memory leak in function parse_events_term__sym_hw() > and parse_events_term__clone(). Would you like to add the tag “Fixes” to the commit message? … > +++ b/tools/perf/util/parse-events.c … > @@ -2957,9 +2958,20 @@ int parse_events_term__sym_hw(struct > parse_events_term **term

Re: [PATCH v5] virtio_vsock: Fix race condition in virtio_transport_recv_pkt()

2020-05-30 Thread Markus Elfring
> --- > v5: sorry, MIME type in the previous commit message > > net/vmw_vsock/virtio_transport_common.c | 8 Is it helpful to keep the patch version information complete here? (Will another fine-tuning follow for the proposed change?) Regards, Markus

Re: [PATCH v3] virtio_vsock: Fix race condition in virtio_transport_recv_pkt()

2020-05-30 Thread Markus Elfring
> This fixes it by checking sk->sk_shutdown(suggested by Stefano) after > lock_sock since sk->sk_shutdown is set to SHUTDOWN_MASK under the > protection of lock_sock_nested. How do you think about a wording variant like the following? Thus check the data structure member “sk_shutdown” (suggeste

Re: [PATCH] net: atm: Replace kmalloc with kzalloc in the error message

2020-05-29 Thread Markus Elfring
> Looking into the context (atomic!) and error message itself I would rather > drop > message completely. How do you think about to take another look at a previous update suggestion like the following? net/atm: Delete an error message for a failed memory allocation in five functions https://pat

Re: [02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ae_dev()

2020-05-29 Thread Markus Elfring
>> Would you like to add the tag “Fixes” to the commit message? > > Since it seems not a very urgent issue, so i send it to the -next I suggest to take another look at the prioritisation for another completion of the exception handling. > and make it as a code optimization. I propose to reconsi

Re: [02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ae_dev()

2020-05-28 Thread Markus Elfring
>>> Add a mutex destroy call in hclge_init_ae_dev() when fails. >> >> How do you think about a wording variant like the following? … > It looks better. I will try to improve the skill of patch description > and make as many as people can understand the patch. Thanks for your positive feedback. I

Re: [PATCH 02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ad_dev()

2020-05-28 Thread Markus Elfring
> Add a mutex destroy call in hclge_init_ae_dev() when fails. How do you think about a wording variant like the following? Change description: The function “mutex_init” was called before a call of the function “hclge_pci_init”. But the function “mutex_destroy” was not called after ini

Re: [PATCH] qlcnic: Complete exception handling in qlcnic_83xx_interrupt_test()

2020-05-25 Thread Markus Elfring
> …, function qlcnic_83xx_diag_alloc_res() is not handled by > function qlcnic_83xx_diag_free_res() after a call of … I have got understanding difficulties for this wording. > Fix this issue by adding a jump target "fail_mbx_args", > and jump to this new target when qlcnic_alloc_mbx_args() faile

Re: [PATCH v2 4/5] net: devres: provide devm_register_netdev()

2020-05-23 Thread Markus Elfring
… > +++ b/net/devres.c > @@ -38,3 +38,58 @@ struct net_device *devm_alloc_etherdev_mqs(struct device > *dev, int sizeof_priv, … > + * This is a devres variant of register_netdev() for which the unregister > + * function will be call automatically when the managing device is Is the following w

Re: [PATCH] rxrpc: Fix a memory leak in rxkad_verify_response()

2020-05-21 Thread Markus Elfring
> How do you think about a wording variant like the following? > >A ticket was not released after a call of the function “platform_get_irq” > failed. I should have specified an other function name here. A ticket was not released after a call of the function “rxkad_decrypt_ticket” failed.

Re: [PATCH] rxrpc: Fix a memory leak in rxkad_verify_response()

2020-05-21 Thread Markus Elfring
> In function rxkad_verify_response(), pointer "ticket" is not released, > when function rxkad_decrypt_ticket() returns an error, causing a > memory leak bug. I suggest to improve also this change description. How do you think about a wording variant like the following? A ticket was not releas

Re: net/sonic: Software evolution around the application of coding standards

2020-05-12 Thread Markus Elfring
> When the people who write and review the coding standards are the same > people who write and review the code, the standards devolve (given the > prevailing incentives). A coding style is applied also for Linux software. This coding style supports some alternatives for implementation details. De

Re: net/sonic: Fix some resource leaks in error handling paths

2020-05-11 Thread Markus Elfring
> Markus, if you were to write a patch to improve upon coding-style.rst, > who should review it? All involved contributors have got chances to provide constructive comments. I would be curious who will actually dare to contribute further ideas for this area. > If you are unable to write or revi

Re: net/sonic: Fix some resource leaks in error handling paths

2020-05-11 Thread Markus Elfring
>> To which commit would you like to refer to for the proposed adjustment >> of the function “mac_sonic_platform_probe”? > > That was my question to you. We seem to be talking past each other. I have needed a moment to notice your patch as another constructive response for this code review. [PATC

Re: [PATCH v2] net/sonic: Fix some resource leaks in error handling paths

2020-05-11 Thread Markus Elfring
> Changed since v1: > - Improved commit log slightly. > - Changed 'undo_probe1' to 'undo_probe' where appropriate. I find this response interesting. > --- I suggest to replace these triple dashes by a blank line. > drivers/net/ethernet/natsemi/macsonic.c | 17 + > drivers/n

Re: net/sonic: Fix some resource leaks in error handling paths

2020-05-10 Thread Markus Elfring
> If you can't determine when the bug was introduced, I might be able to determine also this information. > how can you criticise a patch for the lack of a Fixes tag? I dared to point two details out for the discussed patch. >> To which commit would you like to refer to for the proposed adjus

Re: net/sonic: Fix some resource leaks in error handling paths

2020-05-10 Thread Markus Elfring
>> https://lore.kernel.org/lkml/20200427061803.53857-1-christophe.jail...@wanadoo.fr/ > > Do you know when these bugs were introduced? I suggest to take another look at a provided tag “Fixes”. To which commit would you like to refer to for the proposed adjustment of the function “mac_sonic_platfor

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

2020-05-09 Thread Markus Elfring
> Is there a way to add a Fixes tag that would not invoke the -stable > process? And was that what you had in mind? Christophe Jaillet proposed to complete the exception handling also for this function implementation. I find that such a software correction is qualified for this tag. https://git.ke

Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths

2020-05-08 Thread Markus Elfring
> While at it, rename a label in order to be slightly more informative and > split some too long lines. Would you like to add the tag “Fixes” to the change description? … > +++ b/drivers/net/ethernet/natsemi/macsonic.c > @@ -506,10 +506,14 @@ static int mac_sonic_platform_probe(struct > platfor

Re: [PATCH] wcn36xx: Fix error handling path in wcn36xx_probe()

2020-05-07 Thread Markus Elfring
… > +++ b/drivers/net/wireless/ath/wcn36xx/main.c … > @@ -1359,6 +1359,8 @@ static int wcn36xx_probe(struct platform_device *pdev) > out_unmap: > iounmap(wcn->ccu_base); > iounmap(wcn->dxe_base); > +out_channel: > + rpmsg_destroy_ept(wcn->smd_channel); > out_wq: > ieee80211_

Re: net: broadcom: fix a mistake about ioremap resource

2020-05-07 Thread Markus Elfring
>> I hope that other contributors can convince you to improve also this >> commit message considerably. >> Would you like to fix the spelling besides other wording weaknesses? > > How about this wording: > > Commit d7a5502b0bb8b ("net: broadcom: convert to > devm_platform_ioremap_resource_byname()"

Re: [v3] nfp: abm: University research groups?

2020-05-07 Thread Markus Elfring
> > I imagined that the bug report (combined with a patch) was triggered by > > an evolving source code analysis approach which will be explained > > in another research paper. Is such a view appropriate? > > https://github.com/umnsec/cheq/ > > Could you elaborate more on "university research group

Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-05 Thread Markus Elfring
> I'm curious if I could still modify these commit message information for the > v1 patch, > which has already been applied and queued up? The maintainer found the provided information good enough. Thus he committed the software correction with the subject “nfp: abm: fix a memory leak bug” on 202

Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-05 Thread Markus Elfring
> What do you think about changing: > "But when nfp_nsp_has_hwinfo_lookup fail, the pointer is not released,.." > to … > or > "But when nfp_nsp_has_hwinfo_lookup fail, I became curious about a related wording variant. But when a call of the function “…” failed, > NSP resource is not cleaned u

Re: net: rtw88: Fix an issue about leaking system resources

2020-05-05 Thread Markus Elfring
> Perhaps we need to find a good researcher student who may do a MD > dissertation out of the case :-) I got the impression that some research is performed already around affected areas. Would you like to reuse any more experiences from there? Regards, Markus

Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-05 Thread Markus Elfring
>> Do you distinguish between releasing items and the role of such a pointer? > I didn't distinguish these.  How do you think about to clarify this aspect a bit more (according to computer science) besides your subsequent constructive feedback? Regards, Markus

Re: [PATCH] net: broadcom: fix a mistake about ioremap resource

2020-05-05 Thread Markus Elfring
> Commit d7a5502b0bb8b ("net: broadcom: convert to > devm_platform_ioremap_resource_byname()") will broke this driver. > idm_base and nicpm_base were optional, after this change, they are > mandatory. it will probe fails with -22 when the dtb doesn't have them > defined. so revert part of this comm

Re: net: rtw88: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
> Brian, Thanks very much for your reminder, Reminders can hopefully trigger positive effects. > These comments have always bothered me. Thanks for such information. > Now I can put it on my blacklist. I find it unfortunate that you choose to adjust your communication preferences in this dir

Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-04 Thread Markus Elfring
> Thanks for your feedback, and yes, I'd like to further adjust the description > details > to make the patch more clear and better. Thanks for such a positive response. > But because Jakub seems to prefer v1, so I'm somehow confused Such a view can be reasonable. The change acceptance varies

Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-04 Thread Markus Elfring
> By the way, is there anything else that I need to improve for this patch? I became curious if you would like to adjust further details from the change description. Other contributors might care less for presented concerns. Regards, Markus

Re: net: rtw88: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
> Markus is not really providing any value to the community. The opinions can vary also around my contributions. > Just search for his recent mail history -- it's all silly commit message > nitpicking of little value. I dare to point questionable implementation (and/or wording) details out. >

Re: net: rtw88: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
> Markus is not really providing any value to the community. The opinions can vary also around my contributions. > Just search for his recent mail history -- it's all silly commit message > nitpicking of little value. I dare to point questionable implementation (and/or wording) details out. >

Re: [PATCH v2] stmmac: fix pointer check after utilization in stmmac_interrupt

2020-05-04 Thread Markus Elfring
> … However, the code fragment is incorrect > because the dev pointer is used before the actual check I find such information interesting. > which leads to undefined behavior. … I suggest to adjust the wording for this “conclusion”. Regards, Markus

Re: [PATCH] net: rtw88: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
>>> the related system resources were not released when pci_iomap() return >>> error in the rtw_pci_io_mapping() function. add pci_release_regions() to >>> fix it. >> >> How do you think about a wording variant like the following? >> >>Subject: >>[PATCH v2] net: rtw88: Complete exception ha

Re: [PATCH] net: enetc: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
> the related system resources were not released when enetc_hw_alloc() > return error in the enetc_pci_mdio_probe(), add iounmap() for error > handling label "err_hw_alloc" to fix it. How do you think about a wording variant like the following? Subject: [PATCH v2] net: enetc: Complete excep

Re: [PATCH] net: rtw88: fix an issue about leak system resources

2020-05-04 Thread Markus Elfring
> the related system resources were not released when pci_iomap() return > error in the rtw_pci_io_mapping() function. add pci_release_regions() to > fix it. How do you think about a wording variant like the following? Subject: [PATCH v2] net: rtw88: Complete exception handling in rtw_pci_i

Re: [PATCH v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-04 Thread Markus Elfring
> But when nfp_nsp_has_hwinfo_lookup fail, the pointer is not released, Do you distinguish between releasing items and the role of such a pointer? > which can lead to a memory leak bug. Would you like to reconsider the usage of the word “can” for such change descriptions? Regards, Markus

Re: [PATCH v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-03 Thread Markus Elfring
> … Thus add a call of the function > “nfp_nsp_close” for the completion of the exception handling. I suggest to mention also the addition of a jump target because of a Linux coding style concern. … > +++ b/drivers/net/ethernet/netronome/nfp/abm/main.c … > @@ -300,12 +297,16 @@ nfp_abm_vnic_set_

Re: [PATCH v2] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()

2020-05-03 Thread Markus Elfring
> … Thus add a call of the function > “nfp_nsp_close” for the completion of the exception handling. Thanks for your positive response. I imagined that a small patch series would be more reasonable than this direct change combination. … > +++ b/drivers/net/ethernet/netronome/nfp/abm/main.c … >

Re: [PATCH] nfp: abm: fix a memory leak bug

2020-05-03 Thread Markus Elfring
… > Fix this issue by adding nfp_nsp_close(nsp) in the error path. How do you think about a wording variant like the following? Subject: [PATCH v2] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac() Change description: … Thus add a call of the function

Re: [PATCH] libertas_tf: Avoid a null pointer dereference in if_usb_disconnect()

2020-05-01 Thread Markus Elfring
> Currently there is a check if priv is null when calling lbtf_remove_card > but not in a previous call to if_usb_reset_dev that can also dereference > priv. Fix this by also only calling lbtf_remove_card if priv is null. I suggest to recheck this description (and the corresponding patch subject)

Re: [PATCH] net/x25: Fix null-ptr-deref in x25_disconnect

2020-04-28 Thread Markus Elfring
> We should check null before do x25_neigh_put in x25_disconnect, > otherwise may cause null-ptr-deref like this: Will it be clearer to use the term “null pointer dereference” in the final commit subject? Regards, Markus

[PATCH v2 0/2] net: phy: mscc-miim: Adjustments for mscc_miim_probe()

2019-09-27 Thread Markus Elfring
From: Markus Elfring Date: Fri, 27 Sep 2019 09:48:09 +0200 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Use devm_platform_ioremap_resource() Move the setting of mii_bus structure members drivers/net/phy/mdio-mscc-miim.c | 19

Re: ss: Checking efficient analysis for network connections

2019-06-13 Thread Markus Elfring
> This whole discussion has zero to do with what text format 'ss' outputs. Would you like to support the clarification of additional data formats for the usage of specific information by tools for socket statistics (from the directory “/proc/net” and related ones)? Regards, Markus

[PATCH] cxgb4/cudbg_lib: Use common error handling code in cudbg_collect_tid()

2018-03-15 Thread SF Markus Elfring
From: Markus Elfring Date: Thu, 15 Mar 2018 14:56:10 +0100 Add a jump target so that a bit of exception handling can be better reused at the end of this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/ethernet/chelsio/cxgb4

Re: [2/2] net/usb/ax88179_178a: Delete three unnecessary variables in ax88179_chk_eee()

2018-03-13 Thread SF Markus Elfring
>> Use three values directly for a condition check without assigning them >> to intermediate variables. > > Hi, > > what is the benefit of this? I proposed a small source code reduction. Other software design directions might become more interesting for this use case. Regards, Markus

[PATCH 3/3] wlcore: Use common error handling code in wl1271_acx_sta_rate_policies()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 22:18:45 +0100 Add a jump target so that a bit of exception handling can be better reused at the end of this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/wireless/ti/wlcore/acx.c

[PATCH 2/3] wlcore: Return directly after a failed kzalloc() in wl1271_acx_sta_rate_policies()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 22:00:31 +0100 Return directly after a call of the function "kzalloc" failed at the beginning. Signed-off-by: Markus Elfring --- drivers/net/wireless/ti/wlcore/acx.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git

[PATCH 1/3] wlcore: Delete an unnecessary variable initialisation in wl1271_acx_sta_rate_policies()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 21:51:17 +0100 The local variable "ret" will be set to an appropriate value a bit later. Thus omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- drivers/net/wireless/ti/wlcore/acx.c | 2 +- 1 file

[PATCH 0/3] wlcore: Adjustments for wl1271_acx_sta_rate_policies()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 22:25:45 +0100 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Delete an unnecessary variable initialisation Return directly after a failed kzalloc() Use common error handling code drivers

[PATCH 2/2] net/usb/ax88179_178a: Delete three unnecessary variables in ax88179_chk_eee()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 18:53:28 +0100 Use three values directly for a condition check without assigning them to intermediate variables. Signed-off-by: Markus Elfring --- drivers/net/usb/ax88179_178a.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions

[PATCH 1/2] net/usb/ax88179_178a: Use common code in ax88179_chk_eee()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 18:22:43 +0100 Adjust a jump target so that a bit of common code can be better reused at the end of this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/usb/ax88179_178a.c | 34

[PATCH 0/2] net/usb/ax88179_178a: Adjustments for ax88179_chk_eee()

2018-03-10 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 10 Mar 2018 19:05:45 +0100 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Use common code Delete three unnecessary variables drivers/net/usb/ax88179_178a.c | 45

Re: hyperv/netvsc: Delete two error messages for a failed memory allocation in netvsc_init_buf()

2018-01-15 Thread SF Markus Elfring
> These messages are not displayed anywhere else: > "unable to allocate receive buffer of size %u\n" > "unable to allocate send buffer of size %u\n", > > After set ret = -ENOMEM; and cleanup, we won't know which buffer allocation > failed without the error message. How do you think about to achi

[PATCH] sunrpc: Use seq_putc() in unix_gid_show()

2018-01-13 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 13 Jan 2018 20:33:05 +0100 A single character (line break) should be put into a sequence. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- net/sunrpc/svcauth_

[PATCH] l2tp: Use seq_putc() in l2tp_dfs_seq_session_show()

2018-01-13 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 13 Jan 2018 20:11:01 +0100 Two single characters (line breaks) should be put into a sequence. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- net/l2tp/l2tp

Re: hyperv/netvsc: Delete two error messages for a failed memory allocation in netvsc_init_buf()

2018-01-08 Thread SF Markus Elfring
> These messages are not displayed anywhere else: > "unable to allocate receive buffer of size %u\n" > "unable to allocate send buffer of size %u\n", > > After set ret = -ENOMEM; and cleanup, we won't know which buffer allocation > failed without the error message. Do you notice a Linux allocati

Re: [PATCH] atm/clip: Use seq_puts() in svc_addr()

2018-01-07 Thread SF Markus Elfring
>> @@ -708,11 +708,11 @@ static void svc_addr(struct seq_file *seq, struct >> sockaddr_atmsvc *addr) >> static int e164[] = { 1, 8, 4, 6, 1, 0 }; >> >> if (*addr->sas_addr.pub) { >> - seq_printf(seq, "%s", addr->sas_addr.pub); >> + seq_puts(seq, addr->sa

[PATCH] hyperv/netvsc: Delete two error messages for a failed memory allocation in netvsc_init_buf()

2018-01-07 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 7 Jan 2018 21:03:26 +0100 Omit extra messages for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/hyperv/netvsc.c | 5 - 1 file changed, 5 deletions

Re: atm/clip: Use seq_puts() in svc_addr()

2018-01-07 Thread SF Markus Elfring
>> Is the function "seq_puts" a bit more efficient for the desired output >> of a single string in comparison to calling the function "seq_printf" >> for this purpose? > > Will you please be so kind and tell us? How do you think about to get the run time characteristics for these sequence output

Re: atm/clip: Use seq_puts() in svc_addr()

2018-01-07 Thread SF Markus Elfring
>> Two strings should be quickly put into a sequence by two function calls. >> Thus use the function "seq_puts" instead of "seq_printf". >> >> This issue was detected by using the Coccinelle software. > > Can you please explain what the issue really is and what you're trying > to do here? Is the

[PATCH] atm/clip: Use seq_puts() in svc_addr()

2018-01-06 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 6 Jan 2018 22:34:12 +0100 Two strings should be quickly put into a sequence by two function calls. Thus use the function "seq_puts" instead of "seq_printf". This issue was detected by using the Coccinelle software. Signed-off-by: Markus E

Re: bonding: Completion of error handling around bond_update_slave_arr()

2018-01-04 Thread SF Markus Elfring
>>> If you see 8 out of 9 call sites in this file ignore the return value. >> >> How do you think about to fix error detection and corresponding >> exception handling then? >> > If I understand your question correctly - not having memory is not a > correctable error I am unsure if it would be feas

Re: bonding: Completion of error handling around bond_update_slave_arr()

2018-01-04 Thread SF Markus Elfring
> If you see 8 out of 9 call sites in this file ignore the return value. How do you think about to fix error detection and corresponding exception handling then? Regards, Markus

[PATCH 1/2] wireless: libertas_tf: Delete an error message for a failed memory allocation in __if_usb_submit_rx_urb()

2018-01-03 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 3 Jan 2018 20:55:10 +0100 Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/wireless/marvell/libertas_tf/if_usb.c | 1 - 1 file

[PATCH 2/2] wireless: libertas_tf: Improve a size determination in two functions

2018-01-03 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 3 Jan 2018 21:02:17 +0100 Replace the specification of data structures by variable references as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-

[PATCH 0/2] wireless: libertas_tf: Adjustments for three function implementations

2018-01-03 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 3 Jan 2018 21:06:54 +0100 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in __if_usb_submit_rx_urb() Improve a size determination in two functions

[PATCH] net: plip: Delete an error message for a failed memory allocation in plip_receive_packet()

2018-01-03 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 3 Jan 2018 16:00:23 +0100 Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/plip/plip.c | 5 ++--- 1 file changed, 2 insertions(+), 3

[PATCH] ps3_gelic_net: Delete an error message for a failed memory allocation in gelic_descr_prepare_rx()

2018-01-03 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 3 Jan 2018 14:50:59 +0100 Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/ethernet/toshiba/ps3_gelic_net.c | 2 -- 1 file changed, 2

Re: ethernet: mlx4: Delete an error message for a failed memory allocation in five functions

2018-01-03 Thread SF Markus Elfring
> I don't really accept this claim... > Short informative strings worth the tiny space they consume. There can be different opinions for their usefulness. > In addition, some out-of-memory errors are recoverable, even though their > backtrace is also printed. How do you think about to suppress

Re: bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

2018-01-03 Thread SF Markus Elfring
>> Omit an extra message for a memory allocation failure in this function. >> >> This issue was detected by using the Coccinelle software. >> > What is the issue with this message? * Is it redundant? * Would a Linux allocation failure report be already sufficient here? Regards, Markus

[PATCH] netcp_ethss: Delete error messages for a failed memory allocation in three functions

2018-01-02 Thread SF Markus Elfring
From: Markus Elfring Date: Tue, 2 Jan 2018 22:08:50 +0100 Omit extra messages for a memory allocation failure in these functions. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/ethernet/ti/netcp_ethss.c | 27

[PATCH] ethernet: cpsw-phy-sel: Delete an error message for a failed memory allocation in cpsw_phy_sel_probe()

2018-01-02 Thread SF Markus Elfring
From: Markus Elfring Date: Tue, 2 Jan 2018 21:41:25 +0100 Omit an extra message for a memory allocation failure in this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/ethernet/ti/cpsw-phy-sel.c | 4 +--- 1 file changed, 1

  1   2   3   4   5   >