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.

> 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.

> Thanks,
> Sergio

        Thanks for the review !

        Jean

Reply via email to