On 2015/07/02 19:20, Bruce Richardson wrote:
> On Tue, Jun 30, 2015 at 05:24:21PM +0900, Tetsuya Mukawa wrote:
>> From: "Tetsuya.Mukawa" <mukawa at igel.co.jp>
>>
>> This patch fixes below.
>> - bsdapp
>> - Use map_id in pci_uio_map_resource().
>> - Fix interface of pci_map_resource().
>> - Move path variable of mapped_pci_resource structure to pci_map.
>> - linuxapp
>> - Remove redundant error message of linuxapp.
>>
>> 'pci_uio_map_resource()' is implemented in both linuxapp and bsdapp,
>> but interface is different. The patch fixes the function of bsdapp
>> to do same as linuxapp. After applying it, file descriptor should be
>> opened and closed out of pci_map_resource().
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp>
>> ---
>> lib/librte_eal/bsdapp/eal/eal_pci.c | 118
>> ++++++++++++++++++------------
>> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 21 +++---
>> 2 files changed, 80 insertions(+), 59 deletions(-)
>>
> <snip>
>> free_uio_res:
>> + for (i = 0; i < map_idx; i++)
>> + rte_free(maps[i].path);
>> rte_free(uio_res);
>> close_fd:
>> close(dev->intr_handle.fd);
> Comments on previous patch about merging error labels would also apply here.
Right, I will fix it also.
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> index c3b259b..19620fe 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> @@ -116,17 +116,11 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>> fd, (off_t)uio_res->maps[i].offset,
>> (size_t)uio_res->maps[i].size, 0);
>> if (mapaddr != uio_res->maps[i].addr) {
>> - if (mapaddr == MAP_FAILED)
>> - RTE_LOG(ERR, EAL,
>> - "Cannot mmap device
>> resource file %s: %s\n",
>> - uio_res->maps[i].path,
>> - strerror(errno));
>> - else
>> - RTE_LOG(ERR, EAL,
>> - "Cannot mmap device
>> resource file %s to address: %p\n",
>> - uio_res->maps[i].path,
>> - uio_res->maps[i].addr);
>> -
>> + RTE_LOG(ERR, EAL,
>> + "Cannot mmap device resource "
>> + "file %s to address: %p\n",
>> + uio_res->maps[i].path,
>> + uio_res->maps[i].addr);
>> close(fd);
>> return -1;
>> }
>> @@ -353,8 +347,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>
>> /* allocate memory to keep path */
>> maps[map_idx].path = rte_malloc(NULL, strlen(devname) + 1, 0);
>> - if (maps[map_idx].path == NULL)
>> + if (maps[map_idx].path == NULL) {
>> + RTE_LOG(ERR, EAL, "Cannot allocate memory for "
>> + "path: %s\n", strerror(errno));
> It's recommended not to split literal strings across multiple lines. This is
> so that it's easy to find error messages in the source code. In this case,
> because
> of the split, someone using git grep to search for the error message
> "Cannot allocate memory for path"
> would not be able to find it in the code. Longer lines are allowed in code for
> literal strings.
>
> /Bruce
>
Sure, I will fix it.
Tetsuya
