From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5070 invoked by alias); 25 Mar 2011 19:19:54 -0000 Received: (qmail 5060 invoked by uid 22791); 25 Mar 2011 19:19:53 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-gw0-f41.google.com (HELO mail-gw0-f41.google.com) (74.125.83.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Mar 2011 19:19:49 +0000 Received: by gwaa12 with SMTP id a12so720576gwa.0 for ; Fri, 25 Mar 2011 12:19:48 -0700 (PDT) Received: by 10.42.77.7 with SMTP id g7mr1389824ick.147.1301080788097; Fri, 25 Mar 2011 12:19:48 -0700 (PDT) MIME-Version: 1.0 Received: by 10.231.11.195 with HTTP; Fri, 25 Mar 2011 12:19:28 -0700 (PDT) From: Mike Frysinger Date: Fri, 25 Mar 2011 19:32:00 -0000 Message-ID: Subject: Re: [PATCH v3] sim: cfi: new flash device simulation To: "Frank Ch. Eigler" Cc: gdb-patches@sourceware.org, toolchain-devel@blackfin.uclinux.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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: 2011-03/txt/msg01104.txt.bz2 On Fri, Mar 25, 2011 at 11:15 AM, Frank Ch. Eigler wrote: > vapier wrote: >> +/* Clean up any state when this device is removed (e.g. when shutting d= own, >> + =A0 or when reloading via gdb). =A0*/ >> +static void >> +cfi_delete_callback (struct hw *me) >> +{ >> +#ifdef HAVE_MMAP >> + =A0struct cfi *cfi =3D hw_data (me); >> + >> + =A0if (cfi->mmap) >> + =A0 =A0munmap (cfi->mmap, cfi->dev_size); >> +#endif >> +} > > Woudln't you want to write(2) out the contents of the flash backing > store file, if !HAVE_MMAP? perhaps, but that'd require quite a bit more logic that just a single call to write() here. frankly, i dont care about the systems that dont support mmap as required by POSIX. the ifdef's are only to keep from breaking their builds. if someone who actually cares about those systems wants to implement that, then by all means. >> +static void >> +attach_cfi_regs (struct hw *me, struct cfi *cfi) >> +[...] >> + =A0 =A0 =A0cfi->data =3D HW_NALLOC (me, unsigned char, cfi->dev_size); >> + =A0 =A0 =A0if (fd !=3D -1) >> + =A0 =A0 read_len =3D read (fd, cfi->data, cfi->dev_size); > > It's more typical to loop in read(2), in case of an EINTR or somesuch > temporary-failure return code. =A0Or use fread(3)? i didnt bother with the EINTR case as i thought it overkill. i did at least make sure that the code would (for the most part) continue to work in the case of a short read. i wasnt aware that fread() would handle this for me though. i dont mind using that since it'd still be one fread() call. -mike