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@xxxxxxxxx>: > > 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
