From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5084 invoked by alias); 2 Apr 2013 13:30:08 -0000 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 Received: (qmail 4989 invoked by uid 89); 2 Apr 2013 13:30:02 -0000 X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from na3sys009aog118.obsmtp.com (HELO na3sys009aog118.obsmtp.com) (74.125.149.244) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 02 Apr 2013 13:29:58 +0000 Received: from mx10.qnx.com ([209.226.137.110]) (using TLSv1) by na3sys009aob118.postini.com ([74.125.148.12]) with SMTP ID DSNKUVrdURCg/E7XvhGCrbmi785zkj2i6puW@postini.com; Tue, 02 Apr 2013 06:29:58 PDT Received: by mx10.qnx.com (Postfix, from userid 500) id 6B3FD20E46; Tue, 2 Apr 2013 09:29:48 -0400 (EDT) Received: from exhts.ott.qnx.com (exch2 [10.222.2.136]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mx10.qnx.com (Postfix) with ESMTPS id 3975D20E1F; Tue, 2 Apr 2013 09:29:48 -0400 (EDT) Received: from [10.222.96.215] (10.222.2.5) by EXCH2.ott.qnx.com (10.222.2.136) with Microsoft SMTP Server (TLS) id 14.2.318.4; Tue, 2 Apr 2013 09:29:47 -0400 Message-ID: <515ADD4B.9030409@qnx.com> Date: Tue, 02 Apr 2013 13:41:00 -0000 From: Aleksandar Ristovski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 To: Jan Kratochvil CC: "gdb-patches@sourceware.org" Subject: Re: [patch 4/6] Prepare linux_find_memory_regions_full & co. for move References: <51278984.3070208@qnx.com> <20130310210820.GE21130@host2.jankratochvil.net> <514C56CB.4070207@qnx.com> <20130326165242.GA12291@host2.jankratochvil.net> <515353C4.2050203@qnx.com> <20130328202805.GB9375@host2.jankratochvil.net> <5159E384.1040709@qnx.com> <20130402130733.GA11748@host2.jankratochvil.net> In-Reply-To: <20130402130733.GA11748@host2.jankratochvil.net> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-04/txt/msg00034.txt.bz2 On 13-04-02 09:07 AM, Jan Kratochvil wrote: > On Mon, 01 Apr 2013 21:44:04 +0200, Aleksandar Ristovski wrote: > [...] >> --- a/gdb/target.c >> +++ b/gdb/target.c > [...] >> +static LONGEST >> +target_fileio_read_alloc_1 (const char *filename, >> + gdb_byte **buf_p, int padding) >> +{ >> + struct cleanup *close_cleanup; >> + int fd, target_errno; >> + void *memory_to_free = NULL; >> + LONGEST retval; >> + >> + fd = target_fileio_open (filename, FILEIO_O_RDONLY, 0700, &target_errno); >> + if (fd == -1) >> + return -1; >> + >> + close_cleanup = make_cleanup (target_fileio_close_cleanup, &fd); >> + >> + make_cleanup (free_current_contents, &memory_to_free); >> + retval = read_alloc (buf_p, fd, target_fileio_read_alloc_1_pread, padding, >> + &memory_to_free); > > >> + if (retval >= 0) >> + { >> + /* Returned allocated memory is interesting for the caller. */ >> + memory_to_free = NULL; >> } > > BTW ">= 0" is incorrect, the decision of filled in *BUF_P should be "> 0", see > target_fileio_read_stralloc in FSF GDB: > if (transferred < 0) > return NULL; > if (transferred == 0) > return xstrdup (""); > bufstr[transferred] = 0; > or even the documentation of target_fileio_read_alloc: > If a positive value is returned, a > sufficiently large buffer will be allocated using xmalloc and > returned in *BUF_P containing the contents of the object. > > > But I find you wrote a workaround of my bug in the code of read_alloc, the > should have been: > if (n < 0 || (n == 0 && buf_pos == 0)) > xfree (buf); > else > *buf_p = buf; > if (memory_to_free_ptr != NULL) > *memory_to_free_ptr = NULL; > > Then target_fileio_read_alloc_1 can contain just (if you want): > gdb_assert (memory_to_free == NULL); > > Ok, diff from the code at patch 4 (i.e. after applying patches 1-4) that I will incorporate into patch 4 is pasted below. I did not put any assert in target_fileio_read_alloc_1 - I don't think it's useful. Thanks, Aleksandar diff --git a/gdb/target.c b/gdb/target.c index d8ab056..ed0f3bf 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -3522,17 +3522,11 @@ read_alloc (gdb_byte **buf_p, int handle, read_alloc_pre if (n <= 0) { if (n < 0 || (n == 0 && buf_pos == 0)) - { - if (memory_to_free_ptr != NULL) - { - gdb_assert (buf == *memory_to_free_ptr); - *memory_to_free_ptr = NULL; - } - xfree (buf); - } + xfree (buf); else *buf_p = buf; - + if (memory_to_free_ptr != NULL) + *memory_to_free_ptr = NULL; if (n < 0) { /* An error occurred. */ @@ -3576,11 +3570,6 @@ target_fileio_read_alloc_1 (const char *filename, make_cleanup (free_current_contents, &memory_to_free); retval = read_alloc (buf_p, fd, target_fileio_read_alloc_1_pread, padding, &memory_to_free); - if (retval >= 0) - { - /* Returned allocated memory is interesting for the caller. */ - memory_to_free = NULL; - } do_cleanups (close_cleanup); return retval; }