From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH 1/1 v2] eal: Fix misleading error messages,
	errno can't be trusted.
Date: Mon, 03 Oct 2016 22:46:57 +0200
Message-ID: <6928094.Vgjcapsp68@xps13>
References: <20160921211049.GA27472@labs.hpe.com>
 <4a84de40-04b9-da6d-aeb9-ce661a9ebad5@intel.com>
 <20161003155556.GA23662@labs.hpe.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org
To: jean.tourrilhes@hpe.com,
 Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50])
 by dpdk.org (Postfix) with ESMTP id D10E858C8
 for <dev@dpdk.org>; Mon,  3 Oct 2016 22:46:59 +0200 (CEST)
Received: by mail-wm0-f50.google.com with SMTP id p138so173424556wmb.1
 for <dev@dpdk.org>; Mon, 03 Oct 2016 13:46:59 -0700 (PDT)
In-Reply-To: <20161003155556.GA23662@labs.hpe.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-10-03 08:55, Jean Tourrilhes:
> On Mon, Oct 03, 2016 at 02:25:40PM +0100, Sergio Gonzalez Monroy wrote:
> > Hi Jean,
> > 
> > There are some format issues with the patch:
> > 
> > You can run scripts/check-git-log.sh to check them:
> > Wrong headline format:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Wrong headline uppercase:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > Missing 'Fixes' tag:
> >         eal: Fix misleading error messages, errno can't be trusted.
> > 
> > The script's output highlights the different issues.
> 
> 	SOrry about that, I casually read the page on
> http://dpdk.org/dev, but obviously I need to look at it again.

No problem. This guide is more oriented towards regular contributors.
You come with a bug and its fix, we can make some effort to format
the patch :)

The title could be "mem: fix hugepage mapping error messages"

> > On 21/09/2016 22:10, Jean Tourrilhes wrote:
> > >@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
> > >  	mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
> > >  			sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
> > >  			mem_cfg_fd, 0);
> > >+	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
> > >+		if (mem_config != MAP_FAILED)
> > >+			/* errno is stale, don't use */
> > >+			rte_panic("Cannot mmap memory for rte_config at [%p], got [%p] - please use '--base-virtaddr' option\n",
> > >+				  rte_mem_cfg_addr, mem_config);
> > >+		else
> > >+			rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
> > >+				  errno, strerror(errno));
> > >+	}
> > >  	close(mem_cfg_fd);
> > >-	if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
> > >-		rte_panic("Cannot mmap memory for rte_config\n");
> > 
> > NIT but any reason you moved the check before closing the file descriptor?
> > (not that it matters with current code as we panic anyway)
> 
> 	"close()" may change "errno" according to its man page.

Sergio, do you have more comments?
Should we wait another version or is it OK?
Maybe you'd prefer to rework it yourself?