On 03/10/2016 21:46, Thomas Monjalon wrote:
> 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?

Nop.

> Should we wait another version or is it OK?
> Maybe you'd prefer to rework it yourself?

AFAICS it really is just the commit message formatting, so maybe you can 
fix that when applying it?

Sergio

Reply via email to