* [GIT PULL] kbuild updates for v4.7-rc1 @ 2016-05-26 20:33 Michal Marek 2016-05-27 5:26 ` Linus Torvalds 0 siblings, 1 reply; 9+ messages in thread From: Michal Marek @ 2016-05-26 20:33 UTC (permalink / raw) To: Linus Torvalds Cc: arnd, mmarek, mussitantesmortem, nicolas.ferre, nicolas.pitre, robert.jarzmik, yamada.masahiro, linux-kbuild, linux-kernel Hi Linus, please pull these kbuild changes for v4.7-rc1: - New option CONFIG_TRIM_UNUSED_KSYMS which does a two-pass build and unexports symbols which are not used in the current config [Nicolas Pitre] - Several kbuild rule cleanups [Masahiro Yamada] - Warning option adjustments for gcov etc [Arnd Bergmann] - A few more small fixes Thanks, Michal The following changes since commit f55532a0c0b8bb6148f4e07853b876ef73bc69ca: Linux 4.6-rc1 (2016-03-26 16:03:24 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild.git kbuild for you to fetch changes up to c9c6837d39311b0cc14cdbe7c18e815ab44aefb1: kbuild: move -Wunused-const-variable to W=1 warning level (2016-05-11 13:05:40 +0200) ---------------------------------------------------------------- Arnd Bergmann (6): Kbuild: change CC_OPTIMIZE_FOR_SIZE definition Kbuild: disable 'maybe-uninitialized' warning for CONFIG_PROFILE_ALL_BRANCHES gcov: disable for COMPILE_TEST gcov: disable tree-loop-im to reduce stack usage gcov: disable -Wmaybe-uninitialized warning kbuild: move -Wunused-const-variable to W=1 warning level Masahiro Yamada (8): kbuild: drop FORCE from PHONY targets kbuild: specify modules(_install) as PHONY rather than FORCE kbuild: mark help target as PHONY kbuild: delete unnecessary "@:" kbuild: drop redundant "PHONY += FORCE" kbuild: rename cmd_cc_i_c to cmd_cpp_i_c kbuild: rename cmd_as_s_S to cmd_cpp_s_S kbuild: fix if_change and friends to consider argument order Maxim Zhukov (1): scripts: genksyms: fix resource leak Michal Marek (1): kbuild: Get rid of KBUILD_STR Nicolas Ferre (1): kbuild: fix call to adjust_autoksyms.sh when output directory specified Nicolas Pitre (13): kbuild: record needed exported symbols for modules export.h: allow for per-symbol configurable EXPORT_SYMBOL() fixdep: accept extra dependencies on stdin kbuild: de-duplicate fixdep usage kbuild: add fine grained build dependencies for exported symbols kbuild: create/adjust generated/autoksyms.h kbuild: build sample modules along with the rest of the kernel kconfig option for TRIM_UNUSED_KSYMS kbuild: better abstract vmlinux sequential prerequisites kbuild: Fix dependencies for final vmlinux link kbuild: adjust ksym_dep_filter for some cmd_* renames kbuild: fix ksym_dep_filter when multiple EXPORT_SYMBOL() on the same line kbuild: fix adjust_autoksyms.sh for modules that need only one symbol Robert Jarzmik (1): kbuild: forbid kernel directory to contain spaces and colons Makefile | 66 +++++++++++++++------ arch/arm/boot/Makefile | 1 - arch/arm/boot/bootp/Makefile | 3 +- arch/arm/vdso/Makefile | 2 +- arch/h8300/boot/compressed/Makefile | 1 - arch/ia64/Makefile | 6 +- arch/m32r/boot/compressed/Makefile | 1 - arch/mn10300/boot/compressed/Makefile | 1 - arch/nios2/boot/compressed/Makefile | 1 - arch/s390/boot/compressed/Makefile | 1 - arch/sh/boot/compressed/Makefile | 1 - arch/sh/boot/romimage/Makefile | 1 - arch/unicore32/boot/Makefile | 2 +- arch/unicore32/boot/compressed/Makefile | 1 - arch/x86/boot/compressed/Makefile | 1 - arch/x86/entry/vdso/Makefile | 4 +- arch/x86/purgatory/Makefile | 2 - arch/x86/realmode/rm/Makefile | 1 - include/linux/export.h | 33 ++++++++++- init/Kconfig | 29 +++++++++ kernel/gcov/Kconfig | 1 + scripts/Kbuild.include | 45 +++++++++++--- scripts/Makefile.build | 44 +++++++------- scripts/Makefile.extrawarn | 1 + scripts/Makefile.lib | 8 +-- scripts/adjust_autoksyms.sh | 101 ++++++++++++++++++++++++++++++++ scripts/basic/fixdep.c | 61 ++++++++++++++----- scripts/genksyms/genksyms.c | 3 + tools/build/Makefile.build | 8 +-- 29 files changed, 335 insertions(+), 95 deletions(-) create mode 100755 scripts/adjust_autoksyms.sh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-26 20:33 [GIT PULL] kbuild updates for v4.7-rc1 Michal Marek @ 2016-05-27 5:26 ` Linus Torvalds 2016-05-27 12:33 ` Arnd Bergmann 0 siblings, 1 reply; 9+ messages in thread From: Linus Torvalds @ 2016-05-27 5:26 UTC (permalink / raw) To: Michal Marek Cc: Arnd Bergmann, mussitantesmortem, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List On Thu, May 26, 2016 at 1:33 PM, Michal Marek <mmarek@suse.com> wrote: > > git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild.git kbuild This pull results in new warnings. I get new "may be uninitialized" warnings now for me allmodconfig build, and while I didn't look at them all, the one I looked at was just entirely crap: fs/gfs2/dir.c: In function ‘dir_split_leaf.isra.16’: fs/gfs2/dir.c:1021:8: warning: ‘leaf_no’ may be used uninitialized in this function [-Wmaybe-uninitialized] error = get_leaf(dip, leaf_no, &obh); ^ yeah no, leaf_no is initialized a few lines up by error = get_leaf_nr(dip, index, &leaf_no); and the fact that gcc can't follow the trivial error handling is not our fault. It looks *so* trivial that I wonder why. That said, I don't see exactly what in the pull request causes this. My reading of the diff seems to say that you are actually adding *more* cases of -Wno-maybe-uninitialized, not less. So I suspect it's almost accidental in just how the Kconfig option CC_OPTIMIZE_FOR_PERFORMANCE happened, which in turn probably just changes the options for "make allmiodconfig", and it now picks a non-size-optimized build that always showed those warnings and I just didn't see them. Annoying. I've pulled it, but I wish you would look at this. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 5:26 ` Linus Torvalds @ 2016-05-27 12:33 ` Arnd Bergmann 2016-05-27 17:23 ` Linus Torvalds 0 siblings, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2016-05-27 12:33 UTC (permalink / raw) To: Linus Torvalds Cc: Michal Marek, mussitantesmortem, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Thursday, May 26, 2016 10:26:50 PM CEST Linus Torvalds wrote: > On Thu, May 26, 2016 at 1:33 PM, Michal Marek <mmarek@suse.com> wrote: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mmarek/kbuild.git kbuild > > This pull results in new warnings. > > I get new "may be uninitialized" warnings now for me allmodconfig > build, and while I didn't look at them all, the one I looked at was > just entirely crap: > > fs/gfs2/dir.c: In function ‘dir_split_leaf.isra.16’: > fs/gfs2/dir.c:1021:8: warning: ‘leaf_no’ may be used uninitialized > in this function [-Wmaybe-uninitialized] > error = get_leaf(dip, leaf_no, &obh); > ^ > > yeah no, leaf_no is initialized a few lines up by > > error = get_leaf_nr(dip, index, &leaf_no); > > and the fact that gcc can't follow the trivial error handling is not > our fault. It looks *so* trivial that I wonder why. > > That said, I don't see exactly what in the pull request causes this. > > My reading of the diff seems to say that you are actually adding > *more* cases of -Wno-maybe-uninitialized, not less. I put the explanation for this into the individual patch commit logs. > So I suspect it's almost accidental in just how the Kconfig option > CC_OPTIMIZE_FOR_PERFORMANCE happened, which in turn probably just > changes the options for "make allmiodconfig", and it now picks a > non-size-optimized build that always showed those warnings and I just > didn't see them. Correct, the point of the CC_OPTIMIZE_FOR_PERFORMANCE change is that we now see the -Wmaybe-uninitialized warnings by default now. I also submitted patches for all warnings I saw a while ago, but some only happen with certain compiler versions that I don't regularly test on, or are architecture specific. The specific gfs2 warning is one that I fixed before but it came back after some back-and-forth discussions, and I forgot to post it again but kept it in my tree of backlog warning fixes, my mistake. I keep running into new valid Wmaybe-uninitialized warnings on randconfig builds, e.g. https://lkml.org/lkml/2016/5/27/99 today, and having them enabled in allmodconfig should make it easier for patch authors to catch them first. I don't currently get any other -Wmaybe-uninialized warnings using gcc-6 on ARM, but I get two warnings on x86, in drivers/net/wireless/wl3501_cs.c and drivers/net/wireless/intel/iwlwifi/mvm/nvm.c I'll have a look at those. Arnd 8<---- [PATCH] gfs2: avoid maybe-uninitialized warning again gcc can not always figure out which code is only used in an error condition an assignment to indirect argument is only done after the use of IS_ERR() catches errors. In gfs2, this results in a warning about correct code: fs/gfs2/dir.c: In function 'get_first_leaf': fs/gfs2/dir.c:802:9: warning: 'leaf_no' may be used uninitialized in this function [-Wmaybe-uninitialized] fs/gfs2/dir.c: In function 'dir_split_leaf': fs/gfs2/dir.c:1021:8: warning: 'leaf_no' may be used uninitialized in this function [-Wmaybe-uninitialized] I managed to work around this earlier using the IS_ERR_VALUE(), but unfortunately that did not catch all configurations. This new patch reworks the function in question to us PTR_ERR_OR_ZERO() instead, which makes it more obvious to the compiler what happens, and should also result in better object code. Signed-off-by: Arnd Bergmann <arnd@arndb.de> Fixes: 67893f12e537 ("gfs2: avoid uninitialized variable warning") diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c index 4a01f30e9995..271d93905bac 100644 --- a/fs/gfs2/dir.c +++ b/fs/gfs2/dir.c @@ -783,12 +783,15 @@ static int get_leaf_nr(struct gfs2_inode *dip, u32 index, u64 *leaf_out) { __be64 *hash; + int error; hash = gfs2_dir_get_hash_table(dip); - if (IS_ERR(hash)) - return PTR_ERR(hash); - *leaf_out = be64_to_cpu(*(hash + index)); - return 0; + error = PTR_ERR_OR_ZERO(hash); + + if (!error) + *leaf_out = be64_to_cpu(*(hash + index)); + + return error; } static int get_first_leaf(struct gfs2_inode *dip, u32 index, @@ -798,7 +801,7 @@ static int get_first_leaf(struct gfs2_inode *dip, u32 index, int error; error = get_leaf_nr(dip, index, &leaf_no); - if (!IS_ERR_VALUE(error)) + if (!error) error = get_leaf(dip, leaf_no, bh_out); return error; @@ -1014,7 +1017,7 @@ static int dir_split_leaf(struct inode *inode, const struct qstr *name) index = name->hash >> (32 - dip->i_depth); error = get_leaf_nr(dip, index, &leaf_no); - if (IS_ERR_VALUE(error)) + if (error) return error; /* Get the old leaf block */ ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 12:33 ` Arnd Bergmann @ 2016-05-27 17:23 ` Linus Torvalds 2016-05-27 18:28 ` Linus Torvalds 2016-05-27 20:04 ` Arnd Bergmann 0 siblings, 2 replies; 9+ messages in thread From: Linus Torvalds @ 2016-05-27 17:23 UTC (permalink / raw) To: Arnd Bergmann Cc: Michal Marek, mussitantesmortem, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Fri, May 27, 2016 at 5:33 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > > gcc can not always figure out which code is only used in an error > condition an assignment to indirect argument is only done after the > use of IS_ERR() catches errors. In gfs2, this results in a warning > about correct code: I figured out why gcc warns. The code is correct, but gcc isn't able to tell that, because when we return "int" for the error case from get_leaf_nr(), gcc thinks that that *could* be a zero. This code: if (IS_ERR(hash)) return PTR_ERR(hash); is obviously "return non-zero" to a kernel developer (because that's how our error codes work), but to a compiler that "return PTR_ERR()" ends up casting a "long" to an "int" return value. And the compiler thinks that while the "long" was clearly non-zero and negative, the "int" it cast things to might be zero. Yes, a very smart compiler might have figured out that the IS_ERR_VALUE check also means that the value will be non-zero in "int" as well, but that actually takes value range analysis, not just "we already ttested it". And yes, the error goes away if I turn the "int" into "long" into the affected codepaths. I'm not entirely happy about your patch, because I think it makes the code worse. You fix it by effectively making gcc test the resulting value after the type conversion. I'd really prefer to figure out some way to let gcc actually understand this error handling model. Because it's not just the warning, it also generates unnecessary double tests (first comparing a 64-bit value against the error range, and then comparing the truncated 32-bit error code against zero). Making errors be "long" does fix not just the warning but also the code generation. But we've traditionally used "int" for error returns, even though the system call path then again turns it into "long" (for somewhat related reasons, actually - we want to make sure that the "int" error code is valid in all 64 bits). So we *could* just start encouraging people to use "long" for error handling. It would fix warnings and improve the results. But let me think about this a bit more. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 17:23 ` Linus Torvalds @ 2016-05-27 18:28 ` Linus Torvalds 2016-05-27 20:04 ` Arnd Bergmann 1 sibling, 0 replies; 9+ messages in thread From: Linus Torvalds @ 2016-05-27 18:28 UTC (permalink / raw) To: Arnd Bergmann Cc: Michal Marek, Макс Жуков, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Fri, May 27, 2016 at 10:23 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > This code: > > if (IS_ERR(hash)) > return PTR_ERR(hash); > > is obviously "return non-zero" to a kernel developer (because that's > how our error codes work), but to a compiler that "return PTR_ERR()" > ends up casting a "long" to an "int" return value. It doesn't help that gfs2 is using the IS_ERR_VALUE() macro on an "int", whih is bogus to begin with. It happens to *work*, but by the time you've cast an error pointer to "int", you've lost enough bits that "IS_ERR_VALUE()" isn't a valid thing to do. You should just check against zero (possibly against negative). But fixing that and just checking against a non-zero int doesn't actually fix the warning. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 17:23 ` Linus Torvalds 2016-05-27 18:28 ` Linus Torvalds @ 2016-05-27 20:04 ` Arnd Bergmann 2016-05-27 20:20 ` Linus Torvalds 1 sibling, 1 reply; 9+ messages in thread From: Arnd Bergmann @ 2016-05-27 20:04 UTC (permalink / raw) To: Linus Torvalds Cc: Michal Marek, mussitantesmortem, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Friday, May 27, 2016 10:23:00 AM CEST Linus Torvalds wrote: > On Fri, May 27, 2016 at 5:33 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > gcc can not always figure out which code is only used in an error > > condition an assignment to indirect argument is only done after the > > use of IS_ERR() catches errors. In gfs2, this results in a warning > > about correct code: > > I figured out why gcc warns. > > The code is correct, but gcc isn't able to tell that, because when we > return "int" for the error case from get_leaf_nr(), gcc thinks that > that *could* be a zero. > > This code: > > if (IS_ERR(hash)) > return PTR_ERR(hash); > > is obviously "return non-zero" to a kernel developer (because that's > how our error codes work), but to a compiler that "return PTR_ERR()" > ends up casting a "long" to an "int" return value. > > And the compiler thinks that while the "long" was clearly non-zero and > negative, the "int" it cast things to might be zero. Exactly, this was my conclusion back in January when I did the 67893f12e537 patch that fixed it for 32-bit architectures: gcc cannot figure out that 'if (IS_ERR(ptr))' and the '!err' check are equivalent but it does manage to find 'IS_ERR(ptr)' and 'IS_ERR_VAL(PTR_ERR(ptr))' are the same, across multiple inlined functions. > Yes, a very smart compiler might have figured out that the > IS_ERR_VALUE check also means that the value will be non-zero in "int" > as well, but that actually takes value range analysis, not just "we > already ttested it". > > And yes, the error goes away if I turn the "int" into "long" into the > affected codepaths. > > I'm not entirely happy about your patch, because I think it makes the > code worse. You fix it by effectively making gcc test the resulting > value after the type conversion. I'd really prefer to figure out some > way to let gcc actually understand this error handling model. Because > it's not just the warning, it also generates unnecessary double tests > (first comparing a 64-bit value against the error range, and then > comparing the truncated 32-bit error code against zero). I see what you mean. On 32-bit ARM, we get the double compare for any of the versions, while on 64-bit x86, the current version in mainline is the worst, while the original code (with the warning) and the version after my second patch still need the second comparison against zero. > Making errors be "long" does fix not just the warning but also the > code generation. But we've traditionally used "int" for error returns, > even though the system call path then again turns it into "long" (for > somewhat related reasons, actually - we want to make sure that the > "int" error code is valid in all 64 bits). > > So we *could* just start encouraging people to use "long" for error > handling. It would fix warnings and improve the results. But let me > think about this a bit more. > On Friday, May 27, 2016 11:28:48 AM CEST Linus Torvalds wrote: > It doesn't help that gfs2 is using the IS_ERR_VALUE() macro on an > "int", whih is bogus to begin with. It happens to *work*, but by the > time you've cast an error pointer to "int", you've lost enough bits > that "IS_ERR_VALUE()" isn't a valid thing to do. You should just check > against zero (possibly against negative). > > But fixing that and just checking against a non-zero int doesn't > actually fix the warning. Right, removing the IS_ERR_VALUE() gets us back to the state before my original patch, and we get the warning on all architectures, and we certainly don't want to promote the use of IS_ERR_VALUE() further because of the problems you mention with unsigned types that are shorter than 'long'. In fact, the patch that I have in my private tree that was hiding the warning for me on x86 is one that removes all instances of IS_ERR_VALUE() with arguments other than 'unsigned long', see http://pastebin.com/uYa2mkgC for reference. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 20:04 ` Arnd Bergmann @ 2016-05-27 20:20 ` Linus Torvalds 2016-05-27 21:36 ` Arnd Bergmann 2016-05-27 21:52 ` Al Viro 0 siblings, 2 replies; 9+ messages in thread From: Linus Torvalds @ 2016-05-27 20:20 UTC (permalink / raw) To: Arnd Bergmann Cc: Michal Marek, Макс Жуков, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Fri, May 27, 2016 at 1:04 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > In fact, the patch that I have in my private tree that was hiding the > warning for me on x86 is one that removes all instances of IS_ERR_VALUE() > with arguments other than 'unsigned long', see http://pastebin.com/uYa2mkgC > for reference. Please just send me that patch, we need to do this (and then add a cast to pointer (and back to unsigned long) in IS_ERR_VALUE() so that we get warnings for when that macro is mis-used. I didn't look at the details of your patch, but I did look at several IS_ERR_VALUE() uses in the standard kernel, and they were basically all wrong. Even the ones that used it for the rigth reason (vm_brk() that returns a pointer or an error in an "unsigned long") had actively screwed up and truncated that (correct) unsigned long value to "int" before doing the IS_ERR_VALUE(), which made it all wrong again. And the other users just looked entirely bogus, and were all just "zero or negative error code" that has nothing to do with IS_ERR_VALUE(). The code should just check against zero, not use that macro that was designed for something different. Linus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 20:20 ` Linus Torvalds @ 2016-05-27 21:36 ` Arnd Bergmann 2016-05-27 21:52 ` Al Viro 1 sibling, 0 replies; 9+ messages in thread From: Arnd Bergmann @ 2016-05-27 21:36 UTC (permalink / raw) To: Linus Torvalds Cc: Michal Marek, Макс Жуков, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson, Andrzej Hajda On Friday, May 27, 2016 1:20:29 PM CEST Linus Torvalds wrote: > On Fri, May 27, 2016 at 1:04 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > > > In fact, the patch that I have in my private tree that was hiding the > > warning for me on x86 is one that removes all instances of IS_ERR_VALUE() > > with arguments other than 'unsigned long', see http://pastebin.com/uYa2mkgC > > for reference. > > Please just send me that patch, we need to do this (and then add a > cast to pointer (and back to unsigned long) in IS_ERR_VALUE() so that > we get warnings for when that macro is mis-used. Ok, I've tried to come up with a summary of what happened so far on this, and sent the patch your way, leaving out the modified IS_ERR_VALUE() definition for the moment. I've added the people on Cc whose drivers had the most invasive changes. > I didn't look at the details of your patch, but I did look at several > IS_ERR_VALUE() uses in the standard kernel, and they were basically > all wrong. Yes, that matches what we found earlier this year when Andrzej Hajda did some work on fixing the worst problems he found. Arnd ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] kbuild updates for v4.7-rc1 2016-05-27 20:20 ` Linus Torvalds 2016-05-27 21:36 ` Arnd Bergmann @ 2016-05-27 21:52 ` Al Viro 1 sibling, 0 replies; 9+ messages in thread From: Al Viro @ 2016-05-27 21:52 UTC (permalink / raw) To: Linus Torvalds Cc: Arnd Bergmann, Michal Marek, Макс Жуков, nicolas.ferre, Nicolas Pitre, robert.jarzmik, yamada.masahiro, Linux Kbuild mailing list, Linux Kernel Mailing List, Bob Peterson On Fri, May 27, 2016 at 01:20:29PM -0700, Linus Torvalds wrote: > I didn't look at the details of your patch, but I did look at several > IS_ERR_VALUE() uses in the standard kernel, and they were basically > all wrong. Even the ones that used it for the rigth reason (vm_brk() > that returns a pointer or an error in an "unsigned long") had actively > screwed up and truncated that (correct) unsigned long value to "int" > before doing the IS_ERR_VALUE(), which made it all wrong again. > > And the other users just looked entirely bogus, and were all just > "zero or negative error code" that has nothing to do with > IS_ERR_VALUE(). The code should just check against zero, not use that > macro that was designed for something different. Both gfs2 ones and the one in fs/romfs are crap. AFS one is simply ridiculous - result = generic_file_write_iter(iocb, from); if (IS_ERR_VALUE(result)) { _leave(" = %zd", result); return result; } _leave(" = %zd", result); return result; In binfmt*.c some callers are valid (vm_mmap and vm_brk values; BTW, the code in binfmt_flat.c checks for vm_mmap returning 0, which should never happen, AFAICS). Most of binfmt_flat.c ones are pure cargo-culting. net/9p/client.c ones are results of serious mistake in design of 9P.L extensions to 9P - they assume that numeric values of E... are architecture-independent and send them over the wire. The uses of IS_ERR_VALUE there are papering over that. Badly. A couple of sound/* ones should be simply "is negative". netfilter ones are caused by bad calling conventions of xt_percpu_counter_alloc(). I hadn't looked through drivers/* users, but judging by fs/, net/ and sound/, I would expect them to be pointless garbage in the best case and red flags for broken code in the worst. IMO IS_ERR_VALUE() should be renamed to something less generic and be used a lot less than it is right now. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-05-27 21:52 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-26 20:33 [GIT PULL] kbuild updates for v4.7-rc1 Michal Marek 2016-05-27 5:26 ` Linus Torvalds 2016-05-27 12:33 ` Arnd Bergmann 2016-05-27 17:23 ` Linus Torvalds 2016-05-27 18:28 ` Linus Torvalds 2016-05-27 20:04 ` Arnd Bergmann 2016-05-27 20:20 ` Linus Torvalds 2016-05-27 21:36 ` Arnd Bergmann 2016-05-27 21:52 ` Al Viro
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).