From owner-svn-src-all@freebsd.org Thu Feb 8 16:07:03 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 51525F0F676; Thu, 8 Feb 2018 16:07:03 +0000 (UTC) (envelope-from royger@gmail.com) Received: from mail-wm0-x22f.google.com (mail-wm0-x22f.google.com [IPv6:2a00:1450:400c:c09::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A9DFF6EC78; Thu, 8 Feb 2018 16:07:02 +0000 (UTC) (envelope-from royger@gmail.com) Received: by mail-wm0-x22f.google.com with SMTP id v123so10955078wmd.5; Thu, 08 Feb 2018 08:07:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=xJAmm1c4l0VIlgSn2nb7lpyAGQP6xPkvZQTsgWk9jzM=; b=vYzQ7eqJXzKEaXyEjD2tFrBCeZ44DghKfnnihjlO+NN4wqGgpAsw63XjaLH88QIRRC YqV1hwyx9jUzyLYIBYF8KYjhFiGF4vd5DT/449LA/3j9CiMqU7JHKnCOPMobAj2bm5aD +pcMVhPV7Di9uK65DCGHF2imdrblOmK820HuqKJ9DXpqJ9Bw3w0U7mjWV6RSUiw7cxyz cxSKZ8gsZoiLHJgxSXSZsmOmBWrDy6Y67xHwbF8hgNr7mBJPVilvW0ZUcfHEX0Wh26/O GJWZRZPUhzonaCjwf6ZWJ/czLQb6kMYrysQ9f079O2thgQAx/RsmgJ7j4s3zzm0Bu/ZZ +0rQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=xJAmm1c4l0VIlgSn2nb7lpyAGQP6xPkvZQTsgWk9jzM=; b=cHZj0We4TYsZbA7jYOQ8Nz7MD8tLBUgQGRZtTWpsB2BtiL9p0PmdqXaZW/pXeKnxlM 8MhsYB0nafBLKmQUp77+LsCkJjge9lKGcxa2tZkEq9y6A6QCGzUqSr7/2Vo0xGIDGQe0 Q2SoJw2PaQHf3eEiqH86SfmuKRk5F4qdS6L09hdbaG1Rtuq04VYwUW1xB0xtGDJKVSm1 0k02E5gP9mgT0g61G1+qQ1tS7J3KOkHLbyF2pn2gar4jzD2EfN3HN33Ld1Kw9ZU1E0/O QrqdXlr7UiuBcoiDlqFZbl4G6NcnvBqVcLzD3uopmwxYM5qhY+dovx9v/w64g7s45ZyA 5O6w== X-Gm-Message-State: APf1xPBlXrnf8OyW6lJJ+PpoONO8C7KMuOS+zm3q+MlQMTsKaKw8DEig 6y8YcDiXQnLa0417MRGWOfOONw== X-Google-Smtp-Source: AH8x225GNT6KCpZEsWtzTL8m0/MTGCHu8pgsYWfHFtv0BQf/zYgu7rqy+BUC0+dUhKOMb6LzVvFmww== X-Received: by 10.80.186.5 with SMTP id g5mr2224719edc.12.1518106021295; Thu, 08 Feb 2018 08:07:01 -0800 (PST) Received: from localhost (default-46-102-197-194.interdsl.co.uk. [46.102.197.194]) by smtp.gmail.com with ESMTPSA id p32sm122038eda.69.2018.02.08.08.06.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Feb 2018 08:06:59 -0800 (PST) Sender: =?UTF-8?Q?Roger_Pau_Monn=C3=A9?= Date: Thu, 8 Feb 2018 16:06:49 +0000 From: Roger Pau =?iso-8859-1?Q?Monn=E9?= To: Wojciech Macek Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r328536 - in head/stand: common powerpc/kboot Message-ID: <20180208160649.pmkg42i63eipdgiv@MacBook-Pro-de-Roger.local> References: <20180131151823.fwigjbd5uubhshpj@MacBook-Pro-de-Roger.local> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180131151823.fwigjbd5uubhshpj@MacBook-Pro-de-Roger.local> User-Agent: NeoMutt/20171208 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Feb 2018 16:07:03 -0000 Ping? Ed committed a band-aid, but we need to get this fixed. On Wed, Jan 31, 2018 at 03:20:46PM +0000, Roger Pau Monné wrote: > On Mon, Jan 29, 2018 at 09:24:28AM +0000, Wojciech Macek wrote: > > Modified: head/stand/common/load_elf.c > > ============================================================================== > > --- head/stand/common/load_elf.c Mon Jan 29 09:21:08 2018 (r328535) > > +++ head/stand/common/load_elf.c Mon Jan 29 09:24:28 2018 (r328536) > > @@ -29,6 +29,7 @@ > > __FBSDID("$FreeBSD$"); > > > > #include > > +#include > > #include > > #include > > #include > > @@ -118,15 +119,72 @@ __elfN(load_elf_header)(char *filename, elf_file_t ef) > > err = EFTYPE; > > goto error; > > } > > + > > if (ehdr->e_ident[EI_CLASS] != ELF_TARG_CLASS || /* Layout ? */ > > ehdr->e_ident[EI_DATA] != ELF_TARG_DATA || > > So here you force EI_DATA == ELF_TARG_DATA in order to continue... > > > - ehdr->e_ident[EI_VERSION] != EV_CURRENT || /* Version ? */ > > - ehdr->e_version != EV_CURRENT || > > - ehdr->e_machine != ELF_TARG_MACH) { /* Machine ? */ > > + ehdr->e_ident[EI_VERSION] != EV_CURRENT) /* Version ? */ { > > err = EFTYPE; > > goto error; > > } > > > > + /* > > + * Fixup ELF endianness. > > + * > > + * The Xhdr structure was loaded using block read call to > > + * optimize file accesses. It might happen, that the endianness > > + * of the system memory is different that endianness of > > + * the ELF header. > > + * Swap fields here to guarantee that Xhdr always contain > > + * valid data regardless of architecture. > > + */ > > + if (ehdr->e_ident[EI_DATA] == ELFDATA2MSB) { > > + ehdr->e_type = be16toh(ehdr->e_type); > > ... yet here you check for EI_DATA == ELFDATA2MSB which AFAICT it's not > possible given the check above, so the following if branch is dead > code. > > > + ehdr->e_machine = be16toh(ehdr->e_machine); > > + ehdr->e_version = be32toh(ehdr->e_version); > > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) { > > + ehdr->e_entry = be64toh(ehdr->e_entry); > > + ehdr->e_phoff = be64toh(ehdr->e_phoff); > > + ehdr->e_shoff = be64toh(ehdr->e_shoff); > > + } else { > > + ehdr->e_entry = be32toh(ehdr->e_entry); > > + ehdr->e_phoff = be32toh(ehdr->e_phoff); > > + ehdr->e_shoff = be32toh(ehdr->e_shoff); > > + } > > + ehdr->e_flags = be32toh(ehdr->e_flags); > > + ehdr->e_ehsize = be16toh(ehdr->e_ehsize); > > + ehdr->e_phentsize = be16toh(ehdr->e_phentsize); > > + ehdr->e_phnum = be16toh(ehdr->e_phnum); > > + ehdr->e_shentsize = be16toh(ehdr->e_shentsize); > > + ehdr->e_shnum = be16toh(ehdr->e_shnum); > > + ehdr->e_shstrndx = be16toh(ehdr->e_shstrndx); > > + > > + } else { > > + ehdr->e_type = le16toh(ehdr->e_type); > > + ehdr->e_machine = le16toh(ehdr->e_machine); > > + ehdr->e_version = le32toh(ehdr->e_version); > > + if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) { > > + ehdr->e_entry = le64toh(ehdr->e_entry); > > + ehdr->e_phoff = le64toh(ehdr->e_phoff); > > + ehdr->e_shoff = le64toh(ehdr->e_shoff); > > + } else { > > + ehdr->e_entry = le32toh(ehdr->e_entry); > > + ehdr->e_phoff = le32toh(ehdr->e_phoff); > > + ehdr->e_shoff = le32toh(ehdr->e_shoff); > > + } > > + ehdr->e_flags = le32toh(ehdr->e_flags); > > + ehdr->e_ehsize = le16toh(ehdr->e_ehsize); > > + ehdr->e_phentsize = le16toh(ehdr->e_phentsize); > > + ehdr->e_phnum = le16toh(ehdr->e_phnum); > > + ehdr->e_shentsize = le16toh(ehdr->e_shentsize); > > + ehdr->e_shnum = le16toh(ehdr->e_shnum); > > + ehdr->e_shstrndx = le16toh(ehdr->e_shstrndx); > > + } > > I think this chunk (and all the similar ones below) should be put on a > macro in order to avoid such big chunks of code repetition. It's also > fairly easy to forget to change one of the branches in the future. > > Roger. >