From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17324 invoked by alias); 16 Nov 2002 19:16:02 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 17304 invoked from network); 16 Nov 2002 19:16:00 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by sources.redhat.com with SMTP; 16 Nov 2002 19:16:00 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id gAGJFsx31336; Sat, 16 Nov 2002 11:15:54 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] linespec.c, part 5 References: <15829.39563.373999.459749@localhost.redhat.com> From: David Carlton Date: Sat, 16 Nov 2002 11:16:00 -0000 In-Reply-To: <15829.39563.373999.459749@localhost.redhat.com> Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-11/txt/msg00442.txt.bz2 On Fri, 15 Nov 2002 20:08:27 -0500, Elena Zannoni said: > David Carlton writes: >> * The function that this creates, decode_compound, is quite long. >> Later patches in this series will break it down into smaller >> chunks; they will come after I've got decode_line_1 itself down to >> a nice size. > OK but, could you add something to the comment at the top of the > function that describes what the function returns and what the > params are? Sure, I can elaborate on that comment. Though I don't think I want to describe _all_ of the args there: many functions get passed the args of decode_line_1, so I'd rather put a single comment describing that somewhere rather than repeating that over and over again. Maybe I'll replace the these comments right after the end of decode_line_1: /* Now, still more helper functions. */ /* NOTE: carlton/2002-11-07: Some of these have non-obvious side effects. In particular, if a function is passed ARGPTR as an argument, it modifies what ARGPTR points to. (Typically, it advances *ARGPTR past whatever substring it has just looked at.) */ with a comment saying: /* Now, more helper functions for decode_line_1. Some conventions that these functions follow: Decode_line_1 typically passes along some of its arguments or local variables to the subfunctions. It passes the variables by reference if they are modified by the subfunction, and by value otherwise. Some of the functions have side effects that don't arise from variables that are passed by reference. In particular, if a function is passed ARGPTR as an argument, it modifies what ARGPTR points to; typically, it advances *ARGPTR past whatever substring it has just looked at. (If it doesn't modify *ARGPTR, then the function gets passed *ARGPTR instead, which is then called ARG: see set_flags, for example.) Also, functions that return a struct symtabs_and_lines may modify CANONICAL, as in the description of decode_line_1. If a function returns a struct symtabs_and_lines, then that struct will immediately make its way up the call chain to be returned by decode_line_1. In particular, all of the functions decode_XXX calculate the appropriate struct symtabs_and_lines, under the assumption that their argument is of the form XXX. */ Is that clearer? >> * Since decode_line_1 returns the value of decode_compound, there's no >> need to check whether or not decode_compound modifies the arguments >> that it's been passed: later code in decode_line_1 can never be >> effected by such modifications. > Hmmm. maybe I am not understanding what you say. Does > decode_compound modify its args? seems like it changes argptr and > canonical. What if somebody changes decode_line_1 in the future so > that there is more code executed after the call, it could probably > cause some head scratching to see that the call didn't leave things > as expected. I haven't looked too closely though. Does the above comment help? What I meant was: every code path in the code that I extracted leads to either a return from decode_line_1 or an error. So, for example, subsequent code in decode_line_1 won't depend on, say, modifications to 'p' within decode_compound. Decode_compound prepares ARGPTR and CANONICAL appropriately for return, and calculates the struct symtabs_and_lines to be returned. If somebody subsequently decides that, say, a bit more fiddling should happen within decode_line_1, I think these changes should make that fiddling a lot easier rather than a lot harder. It will also help once I've renamed some variables, so 's' turns into 'file_symtab', 'p' turns into 'next_component', and so forth. That will make it much easier for future readers of the code to understand the possible ramifications of changing the values of those variables. (Whereas who knows what it means to change the value of a variable 'p' that is referred to in a zillion places, with some of those uses distinct and some of them not.) David Carlton carlton@math.stanford.edu