From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84862 invoked by alias); 2 Jun 2015 17:17:05 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 84849 invoked by uid 89); 2 Jun 2015 17:17:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 02 Jun 2015 17:17:03 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 76D2919F971; Tue, 2 Jun 2015 17:17:02 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-154.phx2.redhat.com [10.3.113.154]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t52HH1ba025784; Tue, 2 Jun 2015 13:17:02 -0400 Message-ID: <556DE50D.7040303@redhat.com> Date: Tue, 02 Jun 2015 17:33:00 -0000 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: =?windows-1252?Q?Martin_Li=9Aka?= , gcc-patches@gcc.gnu.org Subject: Re: [PATCH 1/2] Memory statistics enhancement. References: <556DB68E.1050807@redhat.com> <556DC634.80103@suse.cz> In-Reply-To: <556DC634.80103@suse.cz> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00223.txt.bz2 On 06/02/2015 09:05 AM, Martin Liška wrote: > On 06/02/2015 03:58 PM, Jeff Law wrote: >> On 06/01/2015 10:16 AM, mliska wrote: >>> Hi. >>> >>> Following 2 patches improve memory statistics infrastructure. First one >>> ports pool allocator to the new infrastructure. And the second one makes >>> column alignment properly. >>> >>> Both can bootstrap on x86_64-linux-pc and survive regression tests. >>> >>> Ready for trunk? >>> Thank you, >>> Martin >>> >>> Port pool-allocator memory stats to a new infrastructure. >>> >>> gcc/ChangeLog: >>> >>> 2015-06-02 Martin Liska >>> >>> * alloc-pool.c (allocate_pool_descriptor): Remove. >>> (struct pool_output_info): Likewise. >>> (print_alloc_pool_statistics): Likewise. >>> (dump_alloc_pool_statistics): Likewise. >>> * alloc-pool.h (struct pool_usage): New struct. >>> (pool_allocator::initialize): Change usage of memory statistics >>> to a new interface. >>> (pool_allocator::release): Likewise. >>> (pool_allocator::allocate): Likewise. >>> (pool_allocator::remove): Likewise. >>> * mem-stats-traits.h (enum mem_alloc_origin): Add new enum value >>> for a pool allocator. >>> * mem-stats.h (struct mem_location): Add new ctor. >>> (struct mem_usage): Add counter for number of >>> instances. >>> (mem_alloc_description::register_descriptor): New overload of >>> the function. >> - >> >>> diff --git a/gcc/alloc-pool.h b/gcc/alloc-pool.h >>> index 96a1342..a1727ce 100644 >>> --- a/gcc/alloc-pool.h >>> +++ b/gcc/alloc-pool.h >> >>> + /* Dump usage coupled to LOC location, where TOTAL is sum of all rows. */ >>> + inline void dump (mem_location *loc, mem_usage &total) const >>> + { >>> + char s[4096]; >>> + sprintf (s, "%s:%i (%s)", loc->get_trimmed_filename (), >>> + loc->m_line, loc->m_function); >> Static sized buffer used in a sprintf where the strings are potentially user controlled. Not good, even in dumping code, still not good. >> >>> + >>> + s[48] = '\0'; >> ?!? Presumably you're just truncating the output line here for the subsequent fprintf call. Consider using a const with a symbolic name rather than the magic "48". I say "consider" because there's magic constants all over the place in the dumping code. So it may not be worth the effort. Your call. >> >> + >>> + /* Dump header with NAME. */ >>> + static inline void dump_header (const char *name) >>> + { >>> + fprintf (stderr, "%-32s%-48s %6s%11s%16s%17s%12s\n", "Pool name", name, >>> + "Pools", "Leak", "Peak", "Times", "Elt size"); >>> + print_dash_line (); >>> + } >>> + >>> + /* Dump footer. */ >>> + inline void dump_footer () >>> + { >>> + print_dash_line (); >>> + fprintf (stderr, "%s%75li%10li\n", "Total", (long)m_instances, >>> + (long)m_allocated); >>> + print_dash_line (); >>> + } >> Note the header is static inline, footer is just inline. Please try to make them consistent. It doesn't look like you did anything with this. Is there a reason that the dump_header and dump_footer have different linkage? Also the linkage/return type for dump_header should be on its own line. With that fixed, this is OK for the trunk. jeff