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?
