From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22530 invoked by alias); 19 Feb 2013 19:11:43 -0000 Received: (qmail 22498 invoked by uid 22791); 19 Feb 2013 19:11:41 -0000 X-SWARE-Spam-Status: No, hits=-8.0 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 19 Feb 2013 19:11:32 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1JJBRJC028575 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 19 Feb 2013 14:11:31 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1JJBQ60006340; Tue, 19 Feb 2013 14:11:27 -0500 Message-ID: <5123CE5E.6090202@redhat.com> Date: Tue, 19 Feb 2013 19:11:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org Subject: Re: [patch] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx References: <20130219184224.GA9706@host2.jankratochvil.net> In-Reply-To: <20130219184224.GA9706@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00516.txt.bz2 On 02/19/2013 06:42 PM, Jan Kratochvil wrote: > ## ----------------------------------------- ## > ## ANSIfy the C compiler whenever possible. ## > ## From Franc,ois Pinard ## > diff --git a/gdb/common/acinclude.m4 b/gdb/common/acinclude.m4 > new file mode 100644 > index 0000000..a4d937d9 > --- /dev/null > +++ b/gdb/common/acinclude.m4 > @@ -0,0 +1,43 @@ > +# Copyright 2012 Free Software Foundation, Inc. 2013 missing. > +dnl GDB_AC_CHECK_VALGRIND > +dnl Check for HAVE_RUNNING_ON_VALGRIND. > +dnl If such symbol is defined is also available. > +AC_DEFUN([GDB_AC_CHECK_VALGRIND], [ > + AC_ARG_WITH([valgrind], > + [AS_HELP_STRING([--without-valgrind], > + [do not include false valgrind warning message skip])],, I have trouble parsing this help string. I suggest instead: "do not include support for running-on-valgrind detection" (Nit, the mmap warning is not really false, as the code is doing what the warning says. It's only that it's annoying.) > + [with_valgrind=check]) > + if test "$with_valgrind" != no; then > + AC_MSG_CHECKING([for RUNNING_ON_VALGRIND macro]) > + AC_CACHE_VAL(gdb_cv_check_running_on_valgrind, [ > + AC_LINK_IFELSE( > + [AC_LANG_PROGRAM([[#include ]], > + [[return RUNNING_ON_VALGRIND;]])], > + [gdb_cv_check_running_on_valgrind=yes], > + [gdb_cv_check_running_on_valgrind=no])]) > + AC_MSG_RESULT($gdb_cv_check_running_on_valgrind) > + if test "$gdb_cv_check_running_on_valgrind" = yes; then > + with_valgrind=yes > + elif test "$with_valgrind" = yes; then > + AC_ERROR([--with-valgrind was requested but it has not been found]) A reader is left confused over what's "it"? A possible simple/small fix is just s/it/support/ or s/it/necessary support/. > > +#ifdef HAVE_RUNNING_ON_VALGRIND > + if (HAVE_RUNNING_ON_VALGRIND) > + return; > +#endif > + I suggest adding a comment above this. Like: /* Below we'll mmap a non-executable page, which under Valgrind results in annoying warnings such as "Bad permissions for mapped region at address 0xfoobar. Just skip the whole test if running under Valgrind. */ > return_address = mmap (NULL, 2, PROT_READ | PROT_WRITE, > MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); > if (return_address == MAP_FAILED) Otherwise this looks good to me. -- Pedro Alves