From: Pavel Shilovsky <piastryyy@gmail.com>
To: Adam Richter <adamrichter4@gmail.com>
Cc: linux-cifs <linux-cifs@vger.kernel.org>
Subject: Re: [PATCH] cifs-utils: smbinfo.c: probably harmless wrong memset sizes + printf format correction
Date: Wed, 7 Aug 2019 14:34:14 -0700 [thread overview]
Message-ID: <CAKywueQEA0gNJ9Xyc7mA7VEHUctWW-CuKS4ei7GHuBb_u6xjkA@mail.gmail.com> (raw)
In-Reply-To: <CAGn-TgiGah++0ibn3vjM+bvBkJa3XttxA12k+Qa4PGME89CTOQ@mail.gmail.com>
Hi Adam,
Sorry it took me a while to look at this. The patch itself looks good
to me. Could you please add an appropriate description, create a patch
with "git format-patch" command and re-send it to the list? This would
allow me to merge it quickly. Submitting a PR on github against the
"next" branch is another good option.
--
Best regards,
Pavel Shilovsky
сб, 25 мая 2019 г. в 15:36, Adam Richter <adamrichter4@gmail.com>:
>
> The attached patch is my attempt at fixing two possibly harmless
> complaints from "cppcheck --enable=warning" from the cifs-utils git
> master branch version of smbinfo.c.
>
> 1. A printf format should have been "%u" instead of "%d" in print_quota.
>
> 2. An incorrect size was being passed to memset in thirteen nearly
> identical places, each using "sizeof(qi)" when "sizeof(*qi)". I am
> not sure but I think these mistakes were probably harmless because the
> memset calls might all be unnecessary and the sizes passed to each
> memset call might never have been larger than it was supposed to be.
>
> Because each of the effected memset calls was immediately preceded by
> the malloc which allocated the data structure and because they each
> ignored the possibility that malloc could fail, I made a function,
> xmalloc0 to combine allocating the memory, zeroing it and exiting with
> a non-zero exit value and a failure message on allocation failure
> (which appears to be a fine way to handle the error in this program).
> The function uses calloc, which could introduce an unnecessary
> multiply, in the hopes that some calloc implementations may avoid the
> memset in the case of freshly allocated memory from mmap, which would
> probably be the case in this program, although I do not know if any
> calloc implementations make this optimization. Anyhow, at least this
> way, the size of the data structure is only computed once in the
> source code.
>
> I realize that these memory allocations may all be for small data
> structures that should be allocated on the stack and also may not need
> to be cleared to all zeroes, but I did not want to delve into coding
> style conventions for stack allocation in the CIFS source tree, and I
> was not 100% certain that clearing the allocated memory was
> unnecessary, although I do see other lines that explicitly initialize
> some field in that that allocated memory to zero. So, please feel
> free to replace my changes with something better or one that involves
> less code churn.
>
> I should also warn that my only testing of these changes was to make
> sure that "cppcheck --enable=warning" no longer complains, that the
> file compiled without complaint (with cifs-utils standard "-Wall
> -Wextra" arguments) and that "./smbinfo quote /dev/null" got past the
> memory allocation to the (correct) ioctl error for /dev/null.
>
> Also, I am not a CIFS developer and this may be the first time I have
> submitted a patch, certainly the first time I remember, so please
> forgive me and feel free to instruct me if I should be following some
> different process to submit this patch.
>
> Thanks in advance for considering this patch submission.
>
> Adam
prev parent reply other threads:[~2019-08-07 21:34 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-25 22:35 [PATCH] cifs-utils: smbinfo.c: probably harmless wrong memset sizes + printf format correction Adam Richter
2019-08-07 21:34 ` Pavel Shilovsky [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKywueQEA0gNJ9Xyc7mA7VEHUctWW-CuKS4ei7GHuBb_u6xjkA@mail.gmail.com \
--to=piastryyy@gmail.com \
--cc=adamrichter4@gmail.com \
--cc=linux-cifs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).