[dpdk-dev] [PATCH v7 05/12] eal: Fix uio mapping differences between linuxapp and bsdapp

[Tetsuya Mukawa]

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