From b469f1e0fc5f0ac882161e627a1255ee11e67c37 Mon Sep 17 00:00:00 2001 From: Jarkko Hietaniemi Date: Sat, 31 May 2014 16:56:00 -0400 Subject: [PATCH] Use PERL_UNUSED_RESULT. (1) Enhance its description. (2) Simplify it: define only if has warn_unused_result. (3) Make it use STMT_START { ... } STMT_END to be less GNU-extensiony. (4) Redo 04783dc7 ("fix 'ignoring return value' compiler warnings") with it. --- doio.c | 13 +++++------- ext/SDBM_File/sdbm/sdbm.c | 5 +++-- mg.c | 52 ++++++++++++++++++++--------------------------- perl.h | 35 ++++++++++++++++++------------- perlio.c | 7 ++----- pp_hot.c | 45 ++++++++++++++++++++-------------------- thread.h | 4 +--- util.c | 4 +--- 8 files changed, 77 insertions(+), 88 deletions(-) diff --git a/doio.c b/doio.c index 49d2269..3a09927 100644 --- a/doio.c +++ b/doio.c @@ -978,16 +978,14 @@ Perl_nextargv(pTHX_ GV *gv) (void)PerlLIO_chmod(PL_oldname,PL_filemode); #endif if (fileuid != PL_statbuf.st_uid || filegid != PL_statbuf.st_gid) { - int rc = 0; + /* XXX silently ignore failures */ #ifdef HAS_FCHOWN - rc = fchown(PL_lastfd,fileuid,filegid); + PERL_UNUSED_RESULT(fchown(PL_lastfd,fileuid,filegid)); #else #ifdef HAS_CHOWN - rc = PerlLIO_chown(PL_oldname,fileuid,filegid); + PERL_UNUSED_RESULT(PerlLIO_chown(PL_oldname,fileuid,filegid)); #endif #endif - /* XXX silently ignore failures */ - PERL_UNUSED_VAR(rc); } } return IoIFP(GvIOp(gv)); @@ -1510,9 +1508,8 @@ S_exec_failed(pTHX_ const char *cmd, int fd, int do_report) Perl_warner(aTHX_ packWARN(WARN_EXEC), "Can't exec \"%s\": %s", cmd, Strerror(e)); if (do_report) { - int rc = PerlLIO_write(fd, (void*)&e, sizeof(int)); - /* silently ignore failures */ - PERL_UNUSED_VAR(rc); + /* XXX silently ignore failures */ + PERL_UNUSED_RESULT(PerlLIO_write(fd, (void*)&e, sizeof(int))); PerlLIO_close(fd); } } diff --git a/ext/SDBM_File/sdbm/sdbm.c b/ext/SDBM_File/sdbm/sdbm.c index f5f893c..5241fea 100644 --- a/ext/SDBM_File/sdbm/sdbm.c +++ b/ext/SDBM_File/sdbm/sdbm.c @@ -379,8 +379,9 @@ makroom(DBM *db, long int hash, int need) */ #ifdef BADMESS rc = write(2, "sdbm: cannot insert after SPLTMAX attempts.\n", 44); - (void)rc; - + /* PERL_UNUSED_VAR() or PERL_UNUSED_RESULT() would be + * useful here but that would mean pulling in perl.h */ + (void)rc; #endif return 0; diff --git a/mg.c b/mg.c index 699c970..58d14c6 100644 --- a/mg.c +++ b/mg.c @@ -2835,7 +2835,7 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) break; case '<': { - int rc = 0; + /* XXX $< currently silently ignores failures */ const Uid_t new_uid = SvUID(sv); PL_delaymagic_uid = new_uid; if (PL_delaymagic) { @@ -2843,34 +2843,32 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) break; /* don't do magic till later */ } #ifdef HAS_SETRUID - rc = setruid(new_uid); + PERL_UNUSED_RESULT(setruid(new_uid)); #else #ifdef HAS_SETREUID - rc = setreuid(new_uid, (Uid_t)-1); + PERL_UNUSED_RESULT(setreuid(new_uid, (Uid_t)-1)); #else #ifdef HAS_SETRESUID - rc = setresuid(new_uid, (Uid_t)-1, (Uid_t)-1); + PERL_UNUSED_RESULT(setresuid(new_uid, (Uid_t)-1, (Uid_t)-1)); #else if (new_uid == PerlProc_geteuid()) { /* special case $< = $> */ #ifdef PERL_DARWIN /* workaround for Darwin's setuid peculiarity, cf [perl #24122] */ if (new_uid != 0 && PerlProc_getuid() == 0) - rc = PerlProc_setuid(0); + PERL_UNUSED_RESULT(PerlProc_setuid(0)); #endif - rc = PerlProc_setuid(new_uid); + PERL_UNUSED_RESULT(PerlProc_setuid(new_uid)); } else { Perl_croak(aTHX_ "setruid() not implemented"); } #endif #endif #endif - /* XXX $< currently silently ignores failures */ - PERL_UNUSED_VAR(rc); break; } case '>': { - int rc = 0; + /* XXX $> currently silently ignores failures */ const Uid_t new_euid = SvUID(sv); PL_delaymagic_euid = new_euid; if (PL_delaymagic) { @@ -2878,29 +2876,27 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) break; /* don't do magic till later */ } #ifdef HAS_SETEUID - rc = seteuid(new_euid); + PERL_UNUSED_RESULT(seteuid(new_euid)); #else #ifdef HAS_SETREUID - rc = setreuid((Uid_t)-1, new_euid); + PERL_UNUSED_RESULT(setreuid((Uid_t)-1, new_euid)); #else #ifdef HAS_SETRESUID - rc = setresuid((Uid_t)-1, new_euid, (Uid_t)-1); + PERL_UNUSED_RESULT(setresuid((Uid_t)-1, new_euid, (Uid_t)-1)); #else if (new_euid == PerlProc_getuid()) /* special case $> = $< */ - rc = PerlProc_setuid(new_euid); + PERL_UNUSED_RESULT(PerlProc_setuid(new_euid)); else { Perl_croak(aTHX_ "seteuid() not implemented"); } #endif #endif #endif - /* XXX $> currently silently ignores failures */ - PERL_UNUSED_VAR(rc); break; } case '(': { - int rc = 0; + /* XXX $( currently silently ignores failures */ const Gid_t new_gid = SvGID(sv); PL_delaymagic_gid = new_gid; if (PL_delaymagic) { @@ -2908,29 +2904,27 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) break; /* don't do magic till later */ } #ifdef HAS_SETRGID - rc = setrgid(new_gid); + PERL_UNUSED_RESULT(setrgid(new_gid)); #else #ifdef HAS_SETREGID - rc = setregid(new_gid, (Gid_t)-1); + PERL_UNUSED_RESULT(setregid(new_gid, (Gid_t)-1)); #else #ifdef HAS_SETRESGID - rc = setresgid(new_gid, (Gid_t)-1, (Gid_t) -1); + PERL_UNUSED_RESULT(setresgid(new_gid, (Gid_t)-1, (Gid_t) -1)); #else if (new_gid == PerlProc_getegid()) /* special case $( = $) */ - rc = PerlProc_setgid(new_gid); + PERL_UNUSED_RESULT(PerlProc_setgid(new_gid)); else { Perl_croak(aTHX_ "setrgid() not implemented"); } #endif #endif #endif - /* XXX $( currently silently ignores failures */ - PERL_UNUSED_VAR(rc); break; } case ')': { - int rc = 0; + /* XXX $) currently silently ignores failures */ Gid_t new_egid; #ifdef HAS_SETGROUPS { @@ -2962,7 +2956,7 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) gary[i] = (Groups_t)Atol(p); } if (i) - rc = setgroups(i, gary); + PERL_UNUSED_RESULT(setgroups(i, gary)); Safefree(gary); } #else /* HAS_SETGROUPS */ @@ -2974,24 +2968,22 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg) break; /* don't do magic till later */ } #ifdef HAS_SETEGID - rc = setegid(new_egid); + PERL_UNUSED_RESULT(setegid(new_egid)); #else #ifdef HAS_SETREGID - rc = setregid((Gid_t)-1, new_egid); + PERL_UNUSED_RESULT(setregid((Gid_t)-1, new_egid)); #else #ifdef HAS_SETRESGID - rc = setresgid((Gid_t)-1, new_egid, (Gid_t)-1); + PERL_UNUSED_RESULT(setresgid((Gid_t)-1, new_egid, (Gid_t)-1)); #else if (new_egid == PerlProc_getgid()) /* special case $) = $( */ - rc = PerlProc_setgid(new_egid); + PERL_UNUSED_RESULT(PerlProc_setgid(new_egid)); else { Perl_croak(aTHX_ "setegid() not implemented"); } #endif #endif #endif - /* XXX $) currently silently ignores failures */ - PERL_UNUSED_VAR(rc); break; } case ':': diff --git a/perl.h b/perl.h index 6dca730..533c253 100644 --- a/perl.h +++ b/perl.h @@ -341,26 +341,33 @@ #endif /* Use PERL_UNUSED_RESULT() to suppress the warnings about unused results - * of function calls, e.g. PERL_UNUSED_RESULT(foo(a, b)). Use it sparingly, - * though, since usually the warning is there for a good reason, - * e.g. for realloc(): the new pointer is not necessarily the old pointer. + * of function calls, e.g. PERL_UNUSED_RESULT(foo(a, b)). + * + * The main reason for this is that the combination of gcc -Wunused-result + * (part of -Wall) and the __attribute__((warn_unused_result)) cannot + * be silenced with casting to void. This causes trouble when the system + * header files use the attribute. + * + * Use PERL_UNUSED_RESULT sparingly, though, since usually the warning + * is there for a good reason: you might lose success/failure information, + * or leak resources, or changes in resources. * * But sometimes you just want to ignore the return value, e.g. on - * codepaths soon ending up in abort, or in "best effort" attempts. - * Sometimes you can capture the return value and use PERL_UNUSED_VAR - * on that. + * codepaths soon ending up in abort, or in "best effort" attempts, + * or in situations where there is no good way to handle failures. * - * The combination of gcc -Wunused-result (part of -Wall) and the gcc - * warn_unused_result attribute cannot be silenced with (void). + * Sometimes PERL_UNUSED_RESULT might not be the most natural way: + * another possibility is that you can capture the return value + * and use PERL_UNUSED_VAR on that. * - * The __typeof__() is unused instead of typeof() since typeof() is - * not available under stricter ANSI modes, and because of compilers - * masquerading as gcc (clang and icc), we want exactly the gcc - * extension __typeof__ and nothing else. + * The __typeof__() is used instead of typeof() since typeof() is not + * available under strict C89, and because of compilers masquerading + * as gcc (clang and icc), we want exactly the gcc extension + * __typeof__ and nothing else. */ #ifndef PERL_UNUSED_RESULT -# if defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN) -# define PERL_UNUSED_RESULT(v) ({ __typeof__(v) z = (v); (void)sizeof(z); }) +# if defined(__GNUC__) && defined(HASATTRIBUTE_WARN_UNUSED_RESULT) +# define PERL_UNUSED_RESULT(v) STMT_START { __typeof__(v) z = (v); (void)sizeof(z); } STMT_END # else # define PERL_UNUSED_RESULT(v) ((void)(v)) # endif diff --git a/perlio.c b/perlio.c index 29c4bf7..c4ace68 100644 --- a/perlio.c +++ b/perlio.c @@ -389,14 +389,13 @@ PerlIO_debug(const char *fmt, ...) } } if (PL_perlio_debug_fd > 0) { - int rc = 0; #ifdef USE_ITHREADS const char * const s = CopFILE(PL_curcop); /* Use fixed buffer as sv_catpvf etc. needs SVs */ char buffer[1024]; const STRLEN len1 = my_snprintf(buffer, sizeof(buffer), "%.40s:%" IVdf " ", s ? s : "(none)", (IV) CopLINE(PL_curcop)); const STRLEN len2 = my_vsnprintf(buffer + len1, sizeof(buffer) - len1, fmt, ap); - rc = PerlLIO_write(PL_perlio_debug_fd, buffer, len1 + len2); + PERL_UNUSED_RESULT(PerlLIO_write(PL_perlio_debug_fd, buffer, len1 + len2)); #else const char *s = CopFILE(PL_curcop); STRLEN len; @@ -405,11 +404,9 @@ PerlIO_debug(const char *fmt, ...) Perl_sv_vcatpvf(aTHX_ sv, fmt, &ap); s = SvPV_const(sv, len); - rc = PerlLIO_write(PL_perlio_debug_fd, s, len); + PERL_UNUSED_RESULT(PerlLIO_write(PL_perlio_debug_fd, s, len)); SvREFCNT_dec(sv); #endif - /* silently ignore failures */ - PERL_UNUSED_VAR(rc); } va_end(ap); } diff --git a/pp_hot.c b/pp_hot.c index e705230..39586b2 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -1182,82 +1182,81 @@ PP(pp_aassign) } } if (UNLIKELY(PL_delaymagic & ~DM_DELAY)) { - int rc = 0; /* Will be used to set PL_tainting below */ Uid_t tmp_uid = PerlProc_getuid(); Uid_t tmp_euid = PerlProc_geteuid(); Gid_t tmp_gid = PerlProc_getgid(); Gid_t tmp_egid = PerlProc_getegid(); + /* XXX $> et al currently silently ignore failures */ if (PL_delaymagic & DM_UID) { #ifdef HAS_SETRESUID - rc = setresuid((PL_delaymagic & DM_RUID) ? PL_delaymagic_uid : (Uid_t)-1, - (PL_delaymagic & DM_EUID) ? PL_delaymagic_euid : (Uid_t)-1, - (Uid_t)-1); + PERL_UNUSED_RESULT( + setresuid((PL_delaymagic & DM_RUID) ? PL_delaymagic_uid : (Uid_t)-1, + (PL_delaymagic & DM_EUID) ? PL_delaymagic_euid : (Uid_t)-1, + (Uid_t)-1)); #else # ifdef HAS_SETREUID - rc = setreuid((PL_delaymagic & DM_RUID) ? PL_delaymagic_uid : (Uid_t)-1, - (PL_delaymagic & DM_EUID) ? PL_delaymagic_euid : (Uid_t)-1); + PERL_UNUSED_RESULT( + setreuid((PL_delaymagic & DM_RUID) ? PL_delaymagic_uid : (Uid_t)-1, + (PL_delaymagic & DM_EUID) ? PL_delaymagic_euid : (Uid_t)-1)); # else # ifdef HAS_SETRUID if ((PL_delaymagic & DM_UID) == DM_RUID) { - rc = setruid(PL_delaymagic_uid); + PERL_UNUSED_RESULT(setruid(PL_delaymagic_uid)); PL_delaymagic &= ~DM_RUID; } # endif /* HAS_SETRUID */ # ifdef HAS_SETEUID if ((PL_delaymagic & DM_UID) == DM_EUID) { - rc = seteuid(PL_delaymagic_euid); + PERL_UNUSED_RESULT(seteuid(PL_delaymagic_euid)); PL_delaymagic &= ~DM_EUID; } # endif /* HAS_SETEUID */ if (PL_delaymagic & DM_UID) { if (PL_delaymagic_uid != PL_delaymagic_euid) DIE(aTHX_ "No setreuid available"); - rc = PerlProc_setuid(PL_delaymagic_uid); + PERL_UNUSED_RESULT(PerlProc_setuid(PL_delaymagic_uid)); } # endif /* HAS_SETREUID */ #endif /* HAS_SETRESUID */ - /* XXX $> et al currently silently ignore failures */ - PERL_UNUSED_VAR(rc); - tmp_uid = PerlProc_getuid(); tmp_euid = PerlProc_geteuid(); } + /* XXX $> et al currently silently ignore failures */ if (PL_delaymagic & DM_GID) { #ifdef HAS_SETRESGID - rc = setresgid((PL_delaymagic & DM_RGID) ? PL_delaymagic_gid : (Gid_t)-1, - (PL_delaymagic & DM_EGID) ? PL_delaymagic_egid : (Gid_t)-1, - (Gid_t)-1); + PERL_UNUSED_RESULT( + setresgid((PL_delaymagic & DM_RGID) ? PL_delaymagic_gid : (Gid_t)-1, + (PL_delaymagic & DM_EGID) ? PL_delaymagic_egid : (Gid_t)-1, + (Gid_t)-1)); #else # ifdef HAS_SETREGID - rc = setregid((PL_delaymagic & DM_RGID) ? PL_delaymagic_gid : (Gid_t)-1, - (PL_delaymagic & DM_EGID) ? PL_delaymagic_egid : (Gid_t)-1); + PERL_UNUSED_RESULT( + setregid((PL_delaymagic & DM_RGID) ? PL_delaymagic_gid : (Gid_t)-1, + (PL_delaymagic & DM_EGID) ? PL_delaymagic_egid : (Gid_t)-1)); # else # ifdef HAS_SETRGID if ((PL_delaymagic & DM_GID) == DM_RGID) { - rc = setrgid(PL_delaymagic_gid); + PERL_UNUSED_RESULT(setrgid(PL_delaymagic_gid)); PL_delaymagic &= ~DM_RGID; } # endif /* HAS_SETRGID */ # ifdef HAS_SETEGID if ((PL_delaymagic & DM_GID) == DM_EGID) { - rc = setegid(PL_delaymagic_egid); + PERL_UNUSED_RESULT(setegid(PL_delaymagic_egid)); PL_delaymagic &= ~DM_EGID; } # endif /* HAS_SETEGID */ if (PL_delaymagic & DM_GID) { if (PL_delaymagic_gid != PL_delaymagic_egid) DIE(aTHX_ "No setregid available"); - rc = PerlProc_setgid(PL_delaymagic_gid); + PERL_UNUSED_RESULT(PerlProc_setgid(PL_delaymagic_gid)); } # endif /* HAS_SETREGID */ #endif /* HAS_SETRESGID */ - /* XXX $> et al currently silently ignore failures */ - PERL_UNUSED_VAR(rc); - tmp_gid = PerlProc_getgid(); tmp_egid = PerlProc_getegid(); } diff --git a/thread.h b/thread.h index cd55635..43932fb 100644 --- a/thread.h +++ b/thread.h @@ -336,9 +336,7 @@ # define ALLOC_THREAD_KEY \ STMT_START { \ if (pthread_key_create(&PL_thr_key, 0)) { \ - int rc; \ - rc = write(2, STR_WITH_LEN("panic: pthread_key_create failed\n")); \ - PERL_UNUSED_VAR(rc); \ + PERL_UNUSED_RESULT(write(2, STR_WITH_LEN("panic: pthread_key_create failed\n"))); \ exit(1); \ } \ } STMT_END diff --git a/util.c b/util.c index 6e71fa2..02c29fa 100644 --- a/util.c +++ b/util.c @@ -1719,9 +1719,7 @@ Perl_croak_no_mem(void) SETERRNO(EBADF,RMS_IFI); else { /* Can't use PerlIO to write as it allocates memory */ - int rc = PerlLIO_write(fd, PL_no_mem, sizeof(PL_no_mem)-1); - /* silently ignore failures */ - PERL_UNUSED_VAR(rc); + PERL_UNUSED_RESULT(PerlLIO_write(fd, PL_no_mem, sizeof(PL_no_mem)-1)); } my_exit(1); } -- 1.8.3.1