From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7782 invoked by alias); 14 Nov 2012 02:57:51 -0000 Received: (qmail 7772 invoked by uid 22791); 14 Nov 2012 02:57:50 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 14 Nov 2012 02:57:43 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 1A1832E09C; Tue, 13 Nov 2012 21:57:43 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id XA1QKME2Kin2; Tue, 13 Nov 2012 21:57:43 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id DA22B2E09B; Tue, 13 Nov 2012 21:57:42 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 7D908C87DF; Tue, 13 Nov 2012 18:57:24 -0800 (PST) Date: Wed, 14 Nov 2012 02:57:00 -0000 From: Joel Brobecker To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH/arm] Backtrace through exception frames on arm/cortex-m target Message-ID: <20121114025724.GC5103@adacore.com> References: <1352816140-3221-1-git-send-email-yao@codesourcery.com> <20121113154926.GB5103@adacore.com> <50A30070.2030700@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50A30070.2030700@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-11/txt/msg00355.txt.bz2 > Sure. We didn't add comments to these functions because they are > installed to 'struct frame_unwind' to compose a unwinder for a specific > type of frames. The situation is similar to gdbarch hook functions, so > I add comment in the similar way, for example, Thank you. I know the comment is slightly superfluous, but a comment like the one you added is good to confirm what it's for, and where to go looking for the function's general documentation. And one of the reasons why I usually insist on documenting every function is because it is just simpler to say "document everything" than to say "document everything except ...". > 2012-11-14 Daniel Jacobowitz > Yao Qi > > * arm-tdep.c (arm_addr_bits_remove): Do not adjust the low > bit of EXC_RETURN. > (arm_m_exception_cache, arm_m_exception_this_id) > (arm_m_exception_prev_register, arm_m_exception_unwind_sniffer) > (arm_m_exception_unwind): New. > (arm_gdbarch_init): Register arm_m_exception_unwind. The patch looks fine to me, except for one minor formatting nit. Pre-appoved with that change. Also, a small reminder that the soft limit for line length is 70 chars, with a hard limit of 80 per an earlier discussion on this list. Some of the comments are a little wide, in that respect, but nothing significant enough to worry about. Just something to keep in mind for the future, if you don't mind. > + /* On M-profile devices, do not strip the low bit from EXC_RETURN > + (the magic exception return address). */ Question from an arm novice: Why? (and would it be something useful to add to the comment as well? > +struct frame_unwind arm_m_exception_unwind = { Can you put the opening curly brace on the next line? We're a little inconsistent regarding this, but I believe that this is the style that we should be using. Thanks, -- Joel