From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24988 invoked by alias); 24 Jan 2008 01:50:32 -0000 Received: (qmail 24896 invoked by uid 22791); 24 Jan 2008 01:50:31 -0000 X-Spam-Check-By: sourceware.org Received: from ug-out-1314.google.com (HELO ug-out-1314.google.com) (66.249.92.169) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 24 Jan 2008 01:50:12 +0000 Received: by ug-out-1314.google.com with SMTP id h2so524352ugf.12 for ; Wed, 23 Jan 2008 17:50:05 -0800 (PST) Received: by 10.67.20.11 with SMTP id x11mr1659480ugi.29.1201139405131; Wed, 23 Jan 2008 17:50:05 -0800 (PST) Received: from ?192.168.0.100? ( [85.241.1.216]) by mx.google.com with ESMTPS id t2sm170387gve.3.2008.01.23.17.50.02 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 23 Jan 2008 17:50:02 -0800 (PST) Message-ID: <4797EEC9.10100@portugalmail.pt> Date: Thu, 24 Jan 2008 01:50:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.13pre) Gecko/20071023 Thunderbird/1.5.0.14pre Mnenhy/0.7.5.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [rfc] Multiple process support in gdbserver References: <20071207212352.GB32256@caradoc.them.org> <20071208152935.GA4888@caradoc.them.org> <20071210153146.GA21899@caradoc.them.org> In-Reply-To: <20071210153146.GA21899@caradoc.them.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2008-01/txt/msg00575.txt.bz2 Hi Daniel, I'm finally looking at this very, very useful patch. I've got a few small comments. + if (remote_exec_file == NULL) + error (_("Running the default executable on the remote target failed; " It seems remote_exec_file can never be NULL. Certainly you wanted to check for empty string. In gdbserver/linux-low.c, when you do this, free (all_processes.head); all_processes.head = all_processes.tail = NULL; Isn't this leaking every ]all_processes.head, all_processes.tail] if (all_processes.head != all_processes.tail) ? shouldn't you be doing something like: for_each_inferiors (&all_processes, free_process); all_processes.head = all_processes.tail = NULL; ? In addition to that, this almost works on Windows targets, we just need to clean up the state when the inferiors are detached or killed, like you're doing on linux. If you prefer, I can post a patch once this goes in. -- Pedro Alves