diff mbox

[dpdk-dev,v2,01/15] eal: Fix cording style of eal_pci.c and eal_pci_uio.c

Message ID 1426155474-1596-2-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tetsuya Mukawa March 12, 2015, 10:17 a.m. UTC 
  This patch fixes cording style of below files in linuxapp and bsdapp.
 - eal_pci.c
 - eal_pci_uio.c

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c       | 67 ++++++++++++++++++-------------
 lib/librte_eal/linuxapp/eal/eal_pci.c     | 32 +++++++++------
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 37 ++++++++++-------
 3 files changed, 80 insertions(+), 56 deletions(-)

Comments

Bruce Richardson March 12, 2015, 10:48 a.m. UTC | #1 
On Thu, Mar 12, 2015 at 07:17:40PM +0900, Tetsuya Mukawa wrote:
> This patch fixes cording style of below files in linuxapp and bsdapp.
>  - eal_pci.c
>  - eal_pci_uio.c
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

Hi Tetsuya,

While there is some good cleanup here, I disagree with a number of the changes
made purely to the whitespace in the file. The style of using a double-indent
for line continuations is very widely used in DPDK code, much more so than the
style of lining things up with the previous line.

So ack to the changes removing unnecessary braces, and occasional splitting of
really long lines (though a few chars over 80 is ok). NAK to the whitespace
and indentation changes.

Regards,
/Bruce

> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 67 ++++++++++++++++++-------------
>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 32 +++++++++------
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 37 ++++++++++-------
>  3 files changed, 80 insertions(+), 56 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index fe3ef86..cbd0a4e 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -142,8 +142,9 @@ pci_map_resource(void *requested_addr, const char *devname, off_t offset,
>  			MAP_SHARED, fd, offset);
>  	close(fd);
>  	if (mapaddr == MAP_FAILED ||
> -			(requested_addr != NULL && mapaddr != requested_addr)) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
> +	    (requested_addr != NULL && mapaddr != requested_addr)) {
> +		RTE_LOG(ERR, EAL,
> +			"%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
>  			" %s (%p)\n", __func__, devname, fd, requested_addr,
>  			(unsigned long)size, (unsigned long)offset,
>  			strerror(errno), mapaddr);
> @@ -161,9 +162,10 @@ fail:
>  static int
>  pci_uio_map_secondary(struct rte_pci_device *dev)
>  {
> -        size_t i;
> -        struct uio_resource *uio_res;
> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> +	size_t i;
> +	struct uio_resource *uio_res;
> +	struct uio_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>  
>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
>  
> @@ -179,10 +181,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>  			    != uio_res->maps[i].addr) {
>  				RTE_LOG(ERR, EAL,
>  					"Cannot mmap device resource\n");
> -				return (-1);
> +				return -1;
>  			}
>  		}
> -		return (0);
> +		return 0;
>  	}
>  
>  	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
> @@ -201,7 +203,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  	uint64_t pagesz;
>  	struct rte_pci_addr *loc = &dev->addr;
>  	struct uio_resource *uio_res;
> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> +	struct uio_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>  	struct uio_map *maps;
>  
>  	dev->intr_handle.fd = -1;
> @@ -209,14 +212,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  
>  	/* secondary processes - use already recorded details */
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> -		return (pci_uio_map_secondary(dev));
> +		return pci_uio_map_secondary(dev);
>  
>  	snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
>  			dev->addr.bus, dev->addr.devid, dev->addr.function);
>  
>  	if (access(devname, O_RDWR) < 0) {
> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
> +		RTE_LOG(WARNING, EAL,
> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
> +			"skipping\n", loc->domain, loc->bus,
> +			loc->devid, loc->function);
>  		return 1;
>  	}
>  
> @@ -233,7 +238,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
>  		RTE_LOG(ERR, EAL,
>  			"%s(): cannot store uio mmap details\n", __func__);
> -		return (-1);
> +		return -1;
>  	}
>  
>  	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
> @@ -248,7 +253,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  
>  		j = uio_res->nb_maps;
>  		/* skip empty BAR */
> -		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
> +		phaddr = dev->mem_resource[i].phys_addr;
> +		if (phaddr == 0)
>  			continue;
>  
>  		/* if matching map is found, then use it */
> @@ -261,7 +267,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  						(size_t)maps[j].size)
>  		    ) == NULL) {
>  			rte_free(uio_res);
> -			return (-1);
> +			return -1;
>  		}
>  
>  		maps[j].addr = mapaddr;
> @@ -271,7 +277,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  
>  	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
>  
> -	return (0);
> +	return 0;
>  }
>  
>  /* Scan one pci sysfs entry, and fill the devices list from it. */
> @@ -311,7 +317,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
>  	/* FreeBSD has no NUMA support (yet) */
>  	dev->numa_node = 0;
>  
> -/* parse resources */
> +	/* parse resources */
>  	switch (conf->pc_hdr & PCIM_HDRTYPE) {
>  	case PCIM_HDRTYPE_NORMAL:
>  		max = PCIR_MAX_BAR_0;
> @@ -440,32 +446,37 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>  
>  		/* check if device's identifiers match the driver's ones */
>  		if (id_table->vendor_id != dev->id.vendor_id &&
> -				id_table->vendor_id != PCI_ANY_ID)
> +		    id_table->vendor_id != PCI_ANY_ID)
>  			continue;
>  		if (id_table->device_id != dev->id.device_id &&
> -				id_table->device_id != PCI_ANY_ID)
> +		    id_table->device_id != PCI_ANY_ID)
>  			continue;
> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> +		if (id_table->subsystem_vendor_id !=
> +		    dev->id.subsystem_vendor_id &&
> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
>  			continue;
> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> -				id_table->subsystem_device_id != PCI_ANY_ID)
> +		if (id_table->subsystem_device_id !=
> +		    dev->id.subsystem_device_id &&
> +		    id_table->subsystem_device_id != PCI_ANY_ID)
>  			continue;
>  
>  		struct rte_pci_addr *loc = &dev->addr;
>  
> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> -				loc->domain, loc->bus, loc->devid, loc->function,
> -				dev->numa_node);
> +		RTE_LOG(DEBUG, EAL,
> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> +			loc->domain, loc->bus, loc->devid, loc->function,
> +			dev->numa_node);
>  
> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> -				dev->id.device_id, dr->name);
> +		RTE_LOG(DEBUG, EAL,
> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> +			dev->id.device_id, dr->name);
>  
>  		/* no initialization when blacklisted, return without error */
>  		if (dev->devargs != NULL &&
>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
>  
> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
> +			RTE_LOG(DEBUG, EAL,
> +				"  Device is blacklisted, not initializing\n");
>  			return 0;
>  		}
>  
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 83c589e..353b0b8 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -154,7 +154,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
>  	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
>  			MAP_SHARED | additional_flags, fd, offset);
>  	if (mapaddr == MAP_FAILED) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
> +		RTE_LOG(ERR, EAL,
> +			"%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
>  			__func__, fd, requested_addr,
>  			(unsigned long)size, (unsigned long)offset,
>  			strerror(errno), mapaddr);
> @@ -631,31 +632,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>  
>  		/* check if device's identifiers match the driver's ones */
>  		if (id_table->vendor_id != dev->id.vendor_id &&
> -				id_table->vendor_id != PCI_ANY_ID)
> +		    id_table->vendor_id != PCI_ANY_ID)
>  			continue;
>  		if (id_table->device_id != dev->id.device_id &&
> -				id_table->device_id != PCI_ANY_ID)
> +		    id_table->device_id != PCI_ANY_ID)
>  			continue;
> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> +		if (id_table->subsystem_vendor_id !=
> +		    dev->id.subsystem_vendor_id &&
> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
>  			continue;
> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> -				id_table->subsystem_device_id != PCI_ANY_ID)
> +		if (id_table->subsystem_device_id !=
> +		    dev->id.subsystem_device_id &&
> +		    id_table->subsystem_device_id != PCI_ANY_ID)
>  			continue;
>  
>  		struct rte_pci_addr *loc = &dev->addr;
>  
> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> -				loc->domain, loc->bus, loc->devid, loc->function,
> -				dev->numa_node);
> +		RTE_LOG(DEBUG, EAL,
> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> +			loc->domain, loc->bus, loc->devid, loc->function,
> +			dev->numa_node);
>  
> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> -				dev->id.device_id, dr->name);
> +		RTE_LOG(DEBUG, EAL,
> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> +			dev->id.device_id, dr->name);
>  
>  		/* no initialization when blacklisted, return without error */
>  		if (dev->devargs != NULL &&
>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
> +			RTE_LOG(DEBUG, EAL,
> +				"  Device is blacklisted, not initializing\n");
>  			return 1;
>  		}
>  
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 2d1c69b..6f229d6 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -92,7 +92,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>  {
>  	int fd, i;
>  	struct mapped_pci_resource *uio_res;
> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> +	struct mapped_pci_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>  
>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
>  
> @@ -117,14 +118,16 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>  			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));
> +						"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);
> +						"Cannot mmap device resource "
> +						"file %s to address: %p\n",
> +						uio_res->maps[i].path,
> +						uio_res->maps[i].addr);
>  
>  				close(fd);
>  				return -1;
> @@ -272,7 +275,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  	uint64_t phaddr;
>  	struct rte_pci_addr *loc = &dev->addr;
>  	struct mapped_pci_resource *uio_res;
> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> +	struct mapped_pci_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>  	struct pci_map *maps;
>  
>  	dev->intr_handle.fd = -1;
> @@ -286,8 +290,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  	/* find uio resource */
>  	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname));
>  	if (uio_num < 0) {
> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
> +		RTE_LOG(WARNING, EAL,
> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
> +			"skipping\n", loc->domain, loc->bus,
> +			loc->devid, loc->function);
>  		return 1;
>  	}
>  	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
> @@ -338,12 +344,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  		if (phaddr == 0)
>  			continue;
>  
> -
>  		/* update devname for mmap  */
>  		snprintf(devname, sizeof(devname),
>  				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
> -				loc->domain, loc->bus, loc->devid, loc->function,
> -				i);
> +				loc->domain, loc->bus, loc->devid,
> +				loc->function, i);
>  
>  		/*
>  		 * open resource file, to mmap it
> @@ -412,7 +417,8 @@ static struct mapped_pci_resource *
>  pci_uio_find_resource(struct rte_pci_device *dev)
>  {
>  	struct mapped_pci_resource *uio_res;
> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> +	struct mapped_pci_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>  
>  	if (dev == NULL)
>  		return NULL;
> @@ -431,7 +437,8 @@ void
>  pci_uio_unmap_resource(struct rte_pci_device *dev)
>  {
>  	struct mapped_pci_resource *uio_res;
> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> +	struct mapped_pci_res_list *uio_res_list =
> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>  
>  	if (dev == NULL)
>  		return;
> -- 
> 1.9.1
>
Michael Qiu March 12, 2015, 10:57 a.m. UTC | #2 
On 3/12/2015 6:50 PM, Bruce Richardson wrote:
> On Thu, Mar 12, 2015 at 07:17:40PM +0900, Tetsuya Mukawa wrote:
>> This patch fixes cording style of below files in linuxapp and bsdapp.
>>  - eal_pci.c
>>  - eal_pci_uio.c
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> Hi Tetsuya,
>
> While there is some good cleanup here, I disagree with a number of the changes
> made purely to the whitespace in the file. The style of using a double-indent
> for line continuations is very widely used in DPDK code, much more so than the
> style of lining things up with the previous line.

Yes, but both style are seeing in dpdk, here the patch is using Tab +
whitespace, which is also
the linux kernel's style.

So is there any rule to allow only one style?
Mixed style is bad...

Thanks,
Michael
> So ack to the changes removing unnecessary braces, and occasional splitting of
> really long lines (though a few chars over 80 is ok). NAK to the whitespace
> and indentation changes.
>
> Regards,
> /Bruce
>
>> ---
>>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 67 ++++++++++++++++++-------------
>>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 32 +++++++++------
>>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 37 ++++++++++-------
>>  3 files changed, 80 insertions(+), 56 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index fe3ef86..cbd0a4e 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> @@ -142,8 +142,9 @@ pci_map_resource(void *requested_addr, const char *devname, off_t offset,
>>  			MAP_SHARED, fd, offset);
>>  	close(fd);
>>  	if (mapaddr == MAP_FAILED ||
>> -			(requested_addr != NULL && mapaddr != requested_addr)) {
>> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
>> +	    (requested_addr != NULL && mapaddr != requested_addr)) {
>> +		RTE_LOG(ERR, EAL,
>> +			"%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
>>  			" %s (%p)\n", __func__, devname, fd, requested_addr,
>>  			(unsigned long)size, (unsigned long)offset,
>>  			strerror(errno), mapaddr);
>> @@ -161,9 +162,10 @@ fail:
>>  static int
>>  pci_uio_map_secondary(struct rte_pci_device *dev)
>>  {
>> -        size_t i;
>> -        struct uio_resource *uio_res;
>> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> +	size_t i;
>> +	struct uio_resource *uio_res;
>> +	struct uio_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>>  
>>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
>>  
>> @@ -179,10 +181,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>>  			    != uio_res->maps[i].addr) {
>>  				RTE_LOG(ERR, EAL,
>>  					"Cannot mmap device resource\n");
>> -				return (-1);
>> +				return -1;
>>  			}
>>  		}
>> -		return (0);
>> +		return 0;
>>  	}
>>  
>>  	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
>> @@ -201,7 +203,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  	uint64_t pagesz;
>>  	struct rte_pci_addr *loc = &dev->addr;
>>  	struct uio_resource *uio_res;
>> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> +	struct uio_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>>  	struct uio_map *maps;
>>  
>>  	dev->intr_handle.fd = -1;
>> @@ -209,14 +212,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  
>>  	/* secondary processes - use already recorded details */
>>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> -		return (pci_uio_map_secondary(dev));
>> +		return pci_uio_map_secondary(dev);
>>  
>>  	snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
>>  			dev->addr.bus, dev->addr.devid, dev->addr.function);
>>  
>>  	if (access(devname, O_RDWR) < 0) {
>> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
>> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
>> +		RTE_LOG(WARNING, EAL,
>> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
>> +			"skipping\n", loc->domain, loc->bus,
>> +			loc->devid, loc->function);
>>  		return 1;
>>  	}
>>  
>> @@ -233,7 +238,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
>>  		RTE_LOG(ERR, EAL,
>>  			"%s(): cannot store uio mmap details\n", __func__);
>> -		return (-1);
>> +		return -1;
>>  	}
>>  
>>  	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
>> @@ -248,7 +253,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  
>>  		j = uio_res->nb_maps;
>>  		/* skip empty BAR */
>> -		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
>> +		phaddr = dev->mem_resource[i].phys_addr;
>> +		if (phaddr == 0)
>>  			continue;
>>  
>>  		/* if matching map is found, then use it */
>> @@ -261,7 +267,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  						(size_t)maps[j].size)
>>  		    ) == NULL) {
>>  			rte_free(uio_res);
>> -			return (-1);
>> +			return -1;
>>  		}
>>  
>>  		maps[j].addr = mapaddr;
>> @@ -271,7 +277,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  
>>  	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
>>  
>> -	return (0);
>> +	return 0;
>>  }
>>  
>>  /* Scan one pci sysfs entry, and fill the devices list from it. */
>> @@ -311,7 +317,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
>>  	/* FreeBSD has no NUMA support (yet) */
>>  	dev->numa_node = 0;
>>  
>> -/* parse resources */
>> +	/* parse resources */
>>  	switch (conf->pc_hdr & PCIM_HDRTYPE) {
>>  	case PCIM_HDRTYPE_NORMAL:
>>  		max = PCIR_MAX_BAR_0;
>> @@ -440,32 +446,37 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>>  
>>  		/* check if device's identifiers match the driver's ones */
>>  		if (id_table->vendor_id != dev->id.vendor_id &&
>> -				id_table->vendor_id != PCI_ANY_ID)
>> +		    id_table->vendor_id != PCI_ANY_ID)
>>  			continue;
>>  		if (id_table->device_id != dev->id.device_id &&
>> -				id_table->device_id != PCI_ANY_ID)
>> +		    id_table->device_id != PCI_ANY_ID)
>>  			continue;
>> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
>> +		if (id_table->subsystem_vendor_id !=
>> +		    dev->id.subsystem_vendor_id &&
>> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
>>  			continue;
>> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>> -				id_table->subsystem_device_id != PCI_ANY_ID)
>> +		if (id_table->subsystem_device_id !=
>> +		    dev->id.subsystem_device_id &&
>> +		    id_table->subsystem_device_id != PCI_ANY_ID)
>>  			continue;
>>  
>>  		struct rte_pci_addr *loc = &dev->addr;
>>  
>> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> -				loc->domain, loc->bus, loc->devid, loc->function,
>> -				dev->numa_node);
>> +		RTE_LOG(DEBUG, EAL,
>> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> +			loc->domain, loc->bus, loc->devid, loc->function,
>> +			dev->numa_node);
>>  
>> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> -				dev->id.device_id, dr->name);
>> +		RTE_LOG(DEBUG, EAL,
>> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> +			dev->id.device_id, dr->name);
>>  
>>  		/* no initialization when blacklisted, return without error */
>>  		if (dev->devargs != NULL &&
>>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
>>  
>> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
>> +			RTE_LOG(DEBUG, EAL,
>> +				"  Device is blacklisted, not initializing\n");
>>  			return 0;
>>  		}
>>  
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> index 83c589e..353b0b8 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> @@ -154,7 +154,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
>>  	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
>>  			MAP_SHARED | additional_flags, fd, offset);
>>  	if (mapaddr == MAP_FAILED) {
>> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
>> +		RTE_LOG(ERR, EAL,
>> +			"%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
>>  			__func__, fd, requested_addr,
>>  			(unsigned long)size, (unsigned long)offset,
>>  			strerror(errno), mapaddr);
>> @@ -631,31 +632,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>>  
>>  		/* check if device's identifiers match the driver's ones */
>>  		if (id_table->vendor_id != dev->id.vendor_id &&
>> -				id_table->vendor_id != PCI_ANY_ID)
>> +		    id_table->vendor_id != PCI_ANY_ID)
>>  			continue;
>>  		if (id_table->device_id != dev->id.device_id &&
>> -				id_table->device_id != PCI_ANY_ID)
>> +		    id_table->device_id != PCI_ANY_ID)
>>  			continue;
>> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
>> +		if (id_table->subsystem_vendor_id !=
>> +		    dev->id.subsystem_vendor_id &&
>> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
>>  			continue;
>> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>> -				id_table->subsystem_device_id != PCI_ANY_ID)
>> +		if (id_table->subsystem_device_id !=
>> +		    dev->id.subsystem_device_id &&
>> +		    id_table->subsystem_device_id != PCI_ANY_ID)
>>  			continue;
>>  
>>  		struct rte_pci_addr *loc = &dev->addr;
>>  
>> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> -				loc->domain, loc->bus, loc->devid, loc->function,
>> -				dev->numa_node);
>> +		RTE_LOG(DEBUG, EAL,
>> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> +			loc->domain, loc->bus, loc->devid, loc->function,
>> +			dev->numa_node);
>>  
>> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> -				dev->id.device_id, dr->name);
>> +		RTE_LOG(DEBUG, EAL,
>> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> +			dev->id.device_id, dr->name);
>>  
>>  		/* no initialization when blacklisted, return without error */
>>  		if (dev->devargs != NULL &&
>>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
>> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
>> +			RTE_LOG(DEBUG, EAL,
>> +				"  Device is blacklisted, not initializing\n");
>>  			return 1;
>>  		}
>>  
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> index 2d1c69b..6f229d6 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> @@ -92,7 +92,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>>  {
>>  	int fd, i;
>>  	struct mapped_pci_resource *uio_res;
>> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> +	struct mapped_pci_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>>  
>>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
>>  
>> @@ -117,14 +118,16 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>>  			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));
>> +						"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);
>> +						"Cannot mmap device resource "
>> +						"file %s to address: %p\n",
>> +						uio_res->maps[i].path,
>> +						uio_res->maps[i].addr);
>>  
>>  				close(fd);
>>  				return -1;
>> @@ -272,7 +275,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  	uint64_t phaddr;
>>  	struct rte_pci_addr *loc = &dev->addr;
>>  	struct mapped_pci_resource *uio_res;
>> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> +	struct mapped_pci_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>>  	struct pci_map *maps;
>>  
>>  	dev->intr_handle.fd = -1;
>> @@ -286,8 +290,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  	/* find uio resource */
>>  	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname));
>>  	if (uio_num < 0) {
>> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
>> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
>> +		RTE_LOG(WARNING, EAL,
>> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
>> +			"skipping\n", loc->domain, loc->bus,
>> +			loc->devid, loc->function);
>>  		return 1;
>>  	}
>>  	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
>> @@ -338,12 +344,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>>  		if (phaddr == 0)
>>  			continue;
>>  
>> -
>>  		/* update devname for mmap  */
>>  		snprintf(devname, sizeof(devname),
>>  				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
>> -				loc->domain, loc->bus, loc->devid, loc->function,
>> -				i);
>> +				loc->domain, loc->bus, loc->devid,
>> +				loc->function, i);
>>  
>>  		/*
>>  		 * open resource file, to mmap it
>> @@ -412,7 +417,8 @@ static struct mapped_pci_resource *
>>  pci_uio_find_resource(struct rte_pci_device *dev)
>>  {
>>  	struct mapped_pci_resource *uio_res;
>> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> +	struct mapped_pci_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>>  
>>  	if (dev == NULL)
>>  		return NULL;
>> @@ -431,7 +437,8 @@ void
>>  pci_uio_unmap_resource(struct rte_pci_device *dev)
>>  {
>>  	struct mapped_pci_resource *uio_res;
>> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> +	struct mapped_pci_res_list *uio_res_list =
>> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>>  
>>  	if (dev == NULL)
>>  		return;
>> -- 
>> 1.9.1
>>
Bruce Richardson March 12, 2015, 11:09 a.m. UTC | #3 
On Thu, Mar 12, 2015 at 10:57:18AM +0000, Qiu, Michael wrote:
> On 3/12/2015 6:50 PM, Bruce Richardson wrote:
> > On Thu, Mar 12, 2015 at 07:17:40PM +0900, Tetsuya Mukawa wrote:
> >> This patch fixes cording style of below files in linuxapp and bsdapp.
> >>  - eal_pci.c
> >>  - eal_pci_uio.c
> >>
> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> > Hi Tetsuya,
> >
> > While there is some good cleanup here, I disagree with a number of the changes
> > made purely to the whitespace in the file. The style of using a double-indent
> > for line continuations is very widely used in DPDK code, much more so than the
> > style of lining things up with the previous line.
> 
> Yes, but both style are seeing in dpdk, here the patch is using Tab +
> whitespace, which is also
> the linux kernel's style.
> 
> So is there any rule to allow only one style?
> Mixed style is bad...
> 
> Thanks,
> Michael

No there is no hard rule, that I am aware of. While I prefer the double-indent
myself, that is beside the point that in the absense of a hard rule to be applied
globally, fixing whitespace from style A to style B just increases the size
of the diff which makes it hard to see the real code changes. Even with a slight
mixing of the styles the code is readable enough as-is.

/Bruce

> > So ack to the changes removing unnecessary braces, and occasional splitting of
> > really long lines (though a few chars over 80 is ok). NAK to the whitespace
> > and indentation changes.
> >
> > Regards,
> > /Bruce
> >
> >> ---
> >>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 67 ++++++++++++++++++-------------
> >>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 32 +++++++++------
> >>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 37 ++++++++++-------
> >>  3 files changed, 80 insertions(+), 56 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> >> index fe3ef86..cbd0a4e 100644
> >> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> >> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> >> @@ -142,8 +142,9 @@ pci_map_resource(void *requested_addr, const char *devname, off_t offset,
> >>  			MAP_SHARED, fd, offset);
> >>  	close(fd);
> >>  	if (mapaddr == MAP_FAILED ||
> >> -			(requested_addr != NULL && mapaddr != requested_addr)) {
> >> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
> >> +	    (requested_addr != NULL && mapaddr != requested_addr)) {
> >> +		RTE_LOG(ERR, EAL,
> >> +			"%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
> >>  			" %s (%p)\n", __func__, devname, fd, requested_addr,
> >>  			(unsigned long)size, (unsigned long)offset,
> >>  			strerror(errno), mapaddr);
> >> @@ -161,9 +162,10 @@ fail:
> >>  static int
> >>  pci_uio_map_secondary(struct rte_pci_device *dev)
> >>  {
> >> -        size_t i;
> >> -        struct uio_resource *uio_res;
> >> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> >> +	size_t i;
> >> +	struct uio_resource *uio_res;
> >> +	struct uio_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> >>  
> >>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
> >>  
> >> @@ -179,10 +181,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
> >>  			    != uio_res->maps[i].addr) {
> >>  				RTE_LOG(ERR, EAL,
> >>  					"Cannot mmap device resource\n");
> >> -				return (-1);
> >> +				return -1;
> >>  			}
> >>  		}
> >> -		return (0);
> >> +		return 0;
> >>  	}
> >>  
> >>  	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
> >> @@ -201,7 +203,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  	uint64_t pagesz;
> >>  	struct rte_pci_addr *loc = &dev->addr;
> >>  	struct uio_resource *uio_res;
> >> -	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> >> +	struct uio_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
> >>  	struct uio_map *maps;
> >>  
> >>  	dev->intr_handle.fd = -1;
> >> @@ -209,14 +212,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  
> >>  	/* secondary processes - use already recorded details */
> >>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >> -		return (pci_uio_map_secondary(dev));
> >> +		return pci_uio_map_secondary(dev);
> >>  
> >>  	snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
> >>  			dev->addr.bus, dev->addr.devid, dev->addr.function);
> >>  
> >>  	if (access(devname, O_RDWR) < 0) {
> >> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
> >> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
> >> +		RTE_LOG(WARNING, EAL,
> >> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
> >> +			"skipping\n", loc->domain, loc->bus,
> >> +			loc->devid, loc->function);
> >>  		return 1;
> >>  	}
> >>  
> >> @@ -233,7 +238,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
> >>  		RTE_LOG(ERR, EAL,
> >>  			"%s(): cannot store uio mmap details\n", __func__);
> >> -		return (-1);
> >> +		return -1;
> >>  	}
> >>  
> >>  	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
> >> @@ -248,7 +253,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  
> >>  		j = uio_res->nb_maps;
> >>  		/* skip empty BAR */
> >> -		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
> >> +		phaddr = dev->mem_resource[i].phys_addr;
> >> +		if (phaddr == 0)
> >>  			continue;
> >>  
> >>  		/* if matching map is found, then use it */
> >> @@ -261,7 +267,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  						(size_t)maps[j].size)
> >>  		    ) == NULL) {
> >>  			rte_free(uio_res);
> >> -			return (-1);
> >> +			return -1;
> >>  		}
> >>  
> >>  		maps[j].addr = mapaddr;
> >> @@ -271,7 +277,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  
> >>  	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
> >>  
> >> -	return (0);
> >> +	return 0;
> >>  }
> >>  
> >>  /* Scan one pci sysfs entry, and fill the devices list from it. */
> >> @@ -311,7 +317,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
> >>  	/* FreeBSD has no NUMA support (yet) */
> >>  	dev->numa_node = 0;
> >>  
> >> -/* parse resources */
> >> +	/* parse resources */
> >>  	switch (conf->pc_hdr & PCIM_HDRTYPE) {
> >>  	case PCIM_HDRTYPE_NORMAL:
> >>  		max = PCIR_MAX_BAR_0;
> >> @@ -440,32 +446,37 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
> >>  
> >>  		/* check if device's identifiers match the driver's ones */
> >>  		if (id_table->vendor_id != dev->id.vendor_id &&
> >> -				id_table->vendor_id != PCI_ANY_ID)
> >> +		    id_table->vendor_id != PCI_ANY_ID)
> >>  			continue;
> >>  		if (id_table->device_id != dev->id.device_id &&
> >> -				id_table->device_id != PCI_ANY_ID)
> >> +		    id_table->device_id != PCI_ANY_ID)
> >>  			continue;
> >> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> >> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> >> +		if (id_table->subsystem_vendor_id !=
> >> +		    dev->id.subsystem_vendor_id &&
> >> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
> >>  			continue;
> >> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> >> -				id_table->subsystem_device_id != PCI_ANY_ID)
> >> +		if (id_table->subsystem_device_id !=
> >> +		    dev->id.subsystem_device_id &&
> >> +		    id_table->subsystem_device_id != PCI_ANY_ID)
> >>  			continue;
> >>  
> >>  		struct rte_pci_addr *loc = &dev->addr;
> >>  
> >> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> >> -				loc->domain, loc->bus, loc->devid, loc->function,
> >> -				dev->numa_node);
> >> +		RTE_LOG(DEBUG, EAL,
> >> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> >> +			loc->domain, loc->bus, loc->devid, loc->function,
> >> +			dev->numa_node);
> >>  
> >> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> >> -				dev->id.device_id, dr->name);
> >> +		RTE_LOG(DEBUG, EAL,
> >> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> >> +			dev->id.device_id, dr->name);
> >>  
> >>  		/* no initialization when blacklisted, return without error */
> >>  		if (dev->devargs != NULL &&
> >>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> >>  
> >> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
> >> +			RTE_LOG(DEBUG, EAL,
> >> +				"  Device is blacklisted, not initializing\n");
> >>  			return 0;
> >>  		}
> >>  
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> index 83c589e..353b0b8 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> >> @@ -154,7 +154,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
> >>  	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
> >>  			MAP_SHARED | additional_flags, fd, offset);
> >>  	if (mapaddr == MAP_FAILED) {
> >> -		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
> >> +		RTE_LOG(ERR, EAL,
> >> +			"%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
> >>  			__func__, fd, requested_addr,
> >>  			(unsigned long)size, (unsigned long)offset,
> >>  			strerror(errno), mapaddr);
> >> @@ -631,31 +632,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
> >>  
> >>  		/* check if device's identifiers match the driver's ones */
> >>  		if (id_table->vendor_id != dev->id.vendor_id &&
> >> -				id_table->vendor_id != PCI_ANY_ID)
> >> +		    id_table->vendor_id != PCI_ANY_ID)
> >>  			continue;
> >>  		if (id_table->device_id != dev->id.device_id &&
> >> -				id_table->device_id != PCI_ANY_ID)
> >> +		    id_table->device_id != PCI_ANY_ID)
> >>  			continue;
> >> -		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> >> -				id_table->subsystem_vendor_id != PCI_ANY_ID)
> >> +		if (id_table->subsystem_vendor_id !=
> >> +		    dev->id.subsystem_vendor_id &&
> >> +		    id_table->subsystem_vendor_id != PCI_ANY_ID)
> >>  			continue;
> >> -		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> >> -				id_table->subsystem_device_id != PCI_ANY_ID)
> >> +		if (id_table->subsystem_device_id !=
> >> +		    dev->id.subsystem_device_id &&
> >> +		    id_table->subsystem_device_id != PCI_ANY_ID)
> >>  			continue;
> >>  
> >>  		struct rte_pci_addr *loc = &dev->addr;
> >>  
> >> -		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> >> -				loc->domain, loc->bus, loc->devid, loc->function,
> >> -				dev->numa_node);
> >> +		RTE_LOG(DEBUG, EAL,
> >> +			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> >> +			loc->domain, loc->bus, loc->devid, loc->function,
> >> +			dev->numa_node);
> >>  
> >> -		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
> >> -				dev->id.device_id, dr->name);
> >> +		RTE_LOG(DEBUG, EAL,
> >> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> >> +			dev->id.device_id, dr->name);
> >>  
> >>  		/* no initialization when blacklisted, return without error */
> >>  		if (dev->devargs != NULL &&
> >>  			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> >> -			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
> >> +			RTE_LOG(DEBUG, EAL,
> >> +				"  Device is blacklisted, not initializing\n");
> >>  			return 1;
> >>  		}
> >>  
> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> index 2d1c69b..6f229d6 100644
> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> >> @@ -92,7 +92,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
> >>  {
> >>  	int fd, i;
> >>  	struct mapped_pci_resource *uio_res;
> >> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >> +	struct mapped_pci_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >>  
> >>  	TAILQ_FOREACH(uio_res, uio_res_list, next) {
> >>  
> >> @@ -117,14 +118,16 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
> >>  			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));
> >> +						"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);
> >> +						"Cannot mmap device resource "
> >> +						"file %s to address: %p\n",
> >> +						uio_res->maps[i].path,
> >> +						uio_res->maps[i].addr);
> >>  
> >>  				close(fd);
> >>  				return -1;
> >> @@ -272,7 +275,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  	uint64_t phaddr;
> >>  	struct rte_pci_addr *loc = &dev->addr;
> >>  	struct mapped_pci_resource *uio_res;
> >> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >> +	struct mapped_pci_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >>  	struct pci_map *maps;
> >>  
> >>  	dev->intr_handle.fd = -1;
> >> @@ -286,8 +290,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  	/* find uio resource */
> >>  	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname));
> >>  	if (uio_num < 0) {
> >> -		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
> >> -				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
> >> +		RTE_LOG(WARNING, EAL,
> >> +			"  "PCI_PRI_FMT" not managed by UIO driver, "
> >> +			"skipping\n", loc->domain, loc->bus,
> >> +			loc->devid, loc->function);
> >>  		return 1;
> >>  	}
> >>  	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
> >> @@ -338,12 +344,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
> >>  		if (phaddr == 0)
> >>  			continue;
> >>  
> >> -
> >>  		/* update devname for mmap  */
> >>  		snprintf(devname, sizeof(devname),
> >>  				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
> >> -				loc->domain, loc->bus, loc->devid, loc->function,
> >> -				i);
> >> +				loc->domain, loc->bus, loc->devid,
> >> +				loc->function, i);
> >>  
> >>  		/*
> >>  		 * open resource file, to mmap it
> >> @@ -412,7 +417,8 @@ static struct mapped_pci_resource *
> >>  pci_uio_find_resource(struct rte_pci_device *dev)
> >>  {
> >>  	struct mapped_pci_resource *uio_res;
> >> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >> +	struct mapped_pci_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >>  
> >>  	if (dev == NULL)
> >>  		return NULL;
> >> @@ -431,7 +437,8 @@ void
> >>  pci_uio_unmap_resource(struct rte_pci_device *dev)
> >>  {
> >>  	struct mapped_pci_resource *uio_res;
> >> -	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >> +	struct mapped_pci_res_list *uio_res_list =
> >> +			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
> >>  
> >>  	if (dev == NULL)
> >>  		return;
> >> -- 
> >> 1.9.1
> >>
> 
>
Tetsuya Mukawa March 13, 2015, 12:25 a.m. UTC | #4 
2015-03-12 20:09 GMT+09:00 Bruce Richardson <bruce.richardson@intel.com>:
> On Thu, Mar 12, 2015 at 10:57:18AM +0000, Qiu, Michael wrote:
>> On 3/12/2015 6:50 PM, Bruce Richardson wrote:
>> > On Thu, Mar 12, 2015 at 07:17:40PM +0900, Tetsuya Mukawa wrote:
>> >> This patch fixes cording style of below files in linuxapp and bsdapp.
>> >>  - eal_pci.c
>> >>  - eal_pci_uio.c
>> >>
>> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> > Hi Tetsuya,
>> >
>> > While there is some good cleanup here, I disagree with a number of the changes
>> > made purely to the whitespace in the file. The style of using a double-indent
>> > for line continuations is very widely used in DPDK code, much more so than the
>> > style of lining things up with the previous line.
>>
>> Yes, but both style are seeing in dpdk, here the patch is using Tab +
>> whitespace, which is also
>> the linux kernel's style.
>>
>> So is there any rule to allow only one style?
>> Mixed style is bad...
>>
>> Thanks,
>> Michael
>
> No there is no hard rule, that I am aware of. While I prefer the double-indent
> myself, that is beside the point that in the absense of a hard rule to be applied
> globally, fixing whitespace from style A to style B just increases the size
> of the diff which makes it hard to see the real code changes. Even with a slight
> mixing of the styles the code is readable enough as-is.
>
> /Bruce

Hi Bruce, Michael,

Thanks for your reviews.
I will fix it, and submit again early next week.

Regards,
Tetsuya

>
>> > So ack to the changes removing unnecessary braces, and occasional splitting of
>> > really long lines (though a few chars over 80 is ok). NAK to the whitespace
>> > and indentation changes.
>> >
>> > Regards,
>> > /Bruce
>> >
>> >> ---
>> >>  lib/librte_eal/bsdapp/eal/eal_pci.c       | 67 ++++++++++++++++++-------------
>> >>  lib/librte_eal/linuxapp/eal/eal_pci.c     | 32 +++++++++------
>> >>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 37 ++++++++++-------
>> >>  3 files changed, 80 insertions(+), 56 deletions(-)
>> >>
>> >> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> >> index fe3ef86..cbd0a4e 100644
>> >> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> >> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> >> @@ -142,8 +142,9 @@ pci_map_resource(void *requested_addr, const char *devname, off_t offset,
>> >>                    MAP_SHARED, fd, offset);
>> >>    close(fd);
>> >>    if (mapaddr == MAP_FAILED ||
>> >> -                  (requested_addr != NULL && mapaddr != requested_addr)) {
>> >> -          RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
>> >> +      (requested_addr != NULL && mapaddr != requested_addr)) {
>> >> +          RTE_LOG(ERR, EAL,
>> >> +                  "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
>> >>                    " %s (%p)\n", __func__, devname, fd, requested_addr,
>> >>                    (unsigned long)size, (unsigned long)offset,
>> >>                    strerror(errno), mapaddr);
>> >> @@ -161,9 +162,10 @@ fail:
>> >>  static int
>> >>  pci_uio_map_secondary(struct rte_pci_device *dev)
>> >>  {
>> >> -        size_t i;
>> >> -        struct uio_resource *uio_res;
>> >> -  struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> >> +  size_t i;
>> >> +  struct uio_resource *uio_res;
>> >> +  struct uio_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> >>
>> >>    TAILQ_FOREACH(uio_res, uio_res_list, next) {
>> >>
>> >> @@ -179,10 +181,10 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>> >>                        != uio_res->maps[i].addr) {
>> >>                            RTE_LOG(ERR, EAL,
>> >>                                    "Cannot mmap device resource\n");
>> >> -                          return (-1);
>> >> +                          return -1;
>> >>                    }
>> >>            }
>> >> -          return (0);
>> >> +          return 0;
>> >>    }
>> >>
>> >>    RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
>> >> @@ -201,7 +203,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>    uint64_t pagesz;
>> >>    struct rte_pci_addr *loc = &dev->addr;
>> >>    struct uio_resource *uio_res;
>> >> -  struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> >> +  struct uio_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>> >>    struct uio_map *maps;
>> >>
>> >>    dev->intr_handle.fd = -1;
>> >> @@ -209,14 +212,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>
>> >>    /* secondary processes - use already recorded details */
>> >>    if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>> >> -          return (pci_uio_map_secondary(dev));
>> >> +          return pci_uio_map_secondary(dev);
>> >>
>> >>    snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
>> >>                    dev->addr.bus, dev->addr.devid, dev->addr.function);
>> >>
>> >>    if (access(devname, O_RDWR) < 0) {
>> >> -          RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
>> >> -                          "skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
>> >> +          RTE_LOG(WARNING, EAL,
>> >> +                  "  "PCI_PRI_FMT" not managed by UIO driver, "
>> >> +                  "skipping\n", loc->domain, loc->bus,
>> >> +                  loc->devid, loc->function);
>> >>            return 1;
>> >>    }
>> >>
>> >> @@ -233,7 +238,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>    if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
>> >>            RTE_LOG(ERR, EAL,
>> >>                    "%s(): cannot store uio mmap details\n", __func__);
>> >> -          return (-1);
>> >> +          return -1;
>> >>    }
>> >>
>> >>    snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
>> >> @@ -248,7 +253,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>
>> >>            j = uio_res->nb_maps;
>> >>            /* skip empty BAR */
>> >> -          if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
>> >> +          phaddr = dev->mem_resource[i].phys_addr;
>> >> +          if (phaddr == 0)
>> >>                    continue;
>> >>
>> >>            /* if matching map is found, then use it */
>> >> @@ -261,7 +267,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>                                            (size_t)maps[j].size)
>> >>                ) == NULL) {
>> >>                    rte_free(uio_res);
>> >> -                  return (-1);
>> >> +                  return -1;
>> >>            }
>> >>
>> >>            maps[j].addr = mapaddr;
>> >> @@ -271,7 +277,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>
>> >>    TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
>> >>
>> >> -  return (0);
>> >> +  return 0;
>> >>  }
>> >>
>> >>  /* Scan one pci sysfs entry, and fill the devices list from it. */
>> >> @@ -311,7 +317,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
>> >>    /* FreeBSD has no NUMA support (yet) */
>> >>    dev->numa_node = 0;
>> >>
>> >> -/* parse resources */
>> >> +  /* parse resources */
>> >>    switch (conf->pc_hdr & PCIM_HDRTYPE) {
>> >>    case PCIM_HDRTYPE_NORMAL:
>> >>            max = PCIR_MAX_BAR_0;
>> >> @@ -440,32 +446,37 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>> >>
>> >>            /* check if device's identifiers match the driver's ones */
>> >>            if (id_table->vendor_id != dev->id.vendor_id &&
>> >> -                          id_table->vendor_id != PCI_ANY_ID)
>> >> +              id_table->vendor_id != PCI_ANY_ID)
>> >>                    continue;
>> >>            if (id_table->device_id != dev->id.device_id &&
>> >> -                          id_table->device_id != PCI_ANY_ID)
>> >> +              id_table->device_id != PCI_ANY_ID)
>> >>                    continue;
>> >> -          if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>> >> -                          id_table->subsystem_vendor_id != PCI_ANY_ID)
>> >> +          if (id_table->subsystem_vendor_id !=
>> >> +              dev->id.subsystem_vendor_id &&
>> >> +              id_table->subsystem_vendor_id != PCI_ANY_ID)
>> >>                    continue;
>> >> -          if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>> >> -                          id_table->subsystem_device_id != PCI_ANY_ID)
>> >> +          if (id_table->subsystem_device_id !=
>> >> +              dev->id.subsystem_device_id &&
>> >> +              id_table->subsystem_device_id != PCI_ANY_ID)
>> >>                    continue;
>> >>
>> >>            struct rte_pci_addr *loc = &dev->addr;
>> >>
>> >> -          RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> >> -                          loc->domain, loc->bus, loc->devid, loc->function,
>> >> -                          dev->numa_node);
>> >> +          RTE_LOG(DEBUG, EAL,
>> >> +                  "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> >> +                  loc->domain, loc->bus, loc->devid, loc->function,
>> >> +                  dev->numa_node);
>> >>
>> >> -          RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> >> -                          dev->id.device_id, dr->name);
>> >> +          RTE_LOG(DEBUG, EAL,
>> >> +                  "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> >> +                  dev->id.device_id, dr->name);
>> >>
>> >>            /* no initialization when blacklisted, return without error */
>> >>            if (dev->devargs != NULL &&
>> >>                    dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
>> >>
>> >> -                  RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
>> >> +                  RTE_LOG(DEBUG, EAL,
>> >> +                          "  Device is blacklisted, not initializing\n");
>> >>                    return 0;
>> >>            }
>> >>
>> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> >> index 83c589e..353b0b8 100644
>> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> >> @@ -154,7 +154,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
>> >>    mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
>> >>                    MAP_SHARED | additional_flags, fd, offset);
>> >>    if (mapaddr == MAP_FAILED) {
>> >> -          RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
>> >> +          RTE_LOG(ERR, EAL,
>> >> +                  "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
>> >>                    __func__, fd, requested_addr,
>> >>                    (unsigned long)size, (unsigned long)offset,
>> >>                    strerror(errno), mapaddr);
>> >> @@ -631,31 +632,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>> >>
>> >>            /* check if device's identifiers match the driver's ones */
>> >>            if (id_table->vendor_id != dev->id.vendor_id &&
>> >> -                          id_table->vendor_id != PCI_ANY_ID)
>> >> +              id_table->vendor_id != PCI_ANY_ID)
>> >>                    continue;
>> >>            if (id_table->device_id != dev->id.device_id &&
>> >> -                          id_table->device_id != PCI_ANY_ID)
>> >> +              id_table->device_id != PCI_ANY_ID)
>> >>                    continue;
>> >> -          if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
>> >> -                          id_table->subsystem_vendor_id != PCI_ANY_ID)
>> >> +          if (id_table->subsystem_vendor_id !=
>> >> +              dev->id.subsystem_vendor_id &&
>> >> +              id_table->subsystem_vendor_id != PCI_ANY_ID)
>> >>                    continue;
>> >> -          if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
>> >> -                          id_table->subsystem_device_id != PCI_ANY_ID)
>> >> +          if (id_table->subsystem_device_id !=
>> >> +              dev->id.subsystem_device_id &&
>> >> +              id_table->subsystem_device_id != PCI_ANY_ID)
>> >>                    continue;
>> >>
>> >>            struct rte_pci_addr *loc = &dev->addr;
>> >>
>> >> -          RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> >> -                          loc->domain, loc->bus, loc->devid, loc->function,
>> >> -                          dev->numa_node);
>> >> +          RTE_LOG(DEBUG, EAL,
>> >> +                  "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
>> >> +                  loc->domain, loc->bus, loc->devid, loc->function,
>> >> +                  dev->numa_node);
>> >>
>> >> -          RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> >> -                          dev->id.device_id, dr->name);
>> >> +          RTE_LOG(DEBUG, EAL,
>> >> +                  "  probe driver: %x:%x %s\n", dev->id.vendor_id,
>> >> +                  dev->id.device_id, dr->name);
>> >>
>> >>            /* no initialization when blacklisted, return without error */
>> >>            if (dev->devargs != NULL &&
>> >>                    dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
>> >> -                  RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
>> >> +                  RTE_LOG(DEBUG, EAL,
>> >> +                          "  Device is blacklisted, not initializing\n");
>> >>                    return 1;
>> >>            }
>> >>
>> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> >> index 2d1c69b..6f229d6 100644
>> >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>> >> @@ -92,7 +92,8 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>> >>  {
>> >>    int fd, i;
>> >>    struct mapped_pci_resource *uio_res;
>> >> -  struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >> +  struct mapped_pci_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >>
>> >>    TAILQ_FOREACH(uio_res, uio_res_list, next) {
>> >>
>> >> @@ -117,14 +118,16 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
>> >>                    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));
>> >> +                                          "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);
>> >> +                                          "Cannot mmap device resource "
>> >> +                                          "file %s to address: %p\n",
>> >> +                                          uio_res->maps[i].path,
>> >> +                                          uio_res->maps[i].addr);
>> >>
>> >>                            close(fd);
>> >>                            return -1;
>> >> @@ -272,7 +275,8 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>    uint64_t phaddr;
>> >>    struct rte_pci_addr *loc = &dev->addr;
>> >>    struct mapped_pci_resource *uio_res;
>> >> -  struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >> +  struct mapped_pci_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >>    struct pci_map *maps;
>> >>
>> >>    dev->intr_handle.fd = -1;
>> >> @@ -286,8 +290,10 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>    /* find uio resource */
>> >>    uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname));
>> >>    if (uio_num < 0) {
>> >> -          RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
>> >> -                          "skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
>> >> +          RTE_LOG(WARNING, EAL,
>> >> +                  "  "PCI_PRI_FMT" not managed by UIO driver, "
>> >> +                  "skipping\n", loc->domain, loc->bus,
>> >> +                  loc->devid, loc->function);
>> >>            return 1;
>> >>    }
>> >>    snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
>> >> @@ -338,12 +344,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>> >>            if (phaddr == 0)
>> >>                    continue;
>> >>
>> >> -
>> >>            /* update devname for mmap  */
>> >>            snprintf(devname, sizeof(devname),
>> >>                            SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
>> >> -                          loc->domain, loc->bus, loc->devid, loc->function,
>> >> -                          i);
>> >> +                          loc->domain, loc->bus, loc->devid,
>> >> +                          loc->function, i);
>> >>
>> >>            /*
>> >>             * open resource file, to mmap it
>> >> @@ -412,7 +417,8 @@ static struct mapped_pci_resource *
>> >>  pci_uio_find_resource(struct rte_pci_device *dev)
>> >>  {
>> >>    struct mapped_pci_resource *uio_res;
>> >> -  struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >> +  struct mapped_pci_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >>
>> >>    if (dev == NULL)
>> >>            return NULL;
>> >> @@ -431,7 +437,8 @@ void
>> >>  pci_uio_unmap_resource(struct rte_pci_device *dev)
>> >>  {
>> >>    struct mapped_pci_resource *uio_res;
>> >> -  struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >> +  struct mapped_pci_res_list *uio_res_list =
>> >> +                  RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
>> >>
>> >>    if (dev == NULL)
>> >>            return;
>> >> --
>> >> 1.9.1
>> >>
>>
>>
diff mbox

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index fe3ef86..cbd0a4e 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -142,8 +142,9 @@  pci_map_resource(void *requested_addr, const char *devname, off_t offset,
 			MAP_SHARED, fd, offset);
 	close(fd);
 	if (mapaddr == MAP_FAILED ||
-			(requested_addr != NULL && mapaddr != requested_addr)) {
-		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
+	    (requested_addr != NULL && mapaddr != requested_addr)) {
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
 			" %s (%p)\n", __func__, devname, fd, requested_addr,
 			(unsigned long)size, (unsigned long)offset,
 			strerror(errno), mapaddr);
@@ -161,9 +162,10 @@  fail:
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
-        size_t i;
-        struct uio_resource *uio_res;
-	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
+	size_t i;
+	struct uio_resource *uio_res;
+	struct uio_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
 
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
 
@@ -179,10 +181,10 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
 					"Cannot mmap device resource\n");
-				return (-1);
+				return -1;
 			}
 		}
-		return (0);
+		return 0;
 	}
 
 	RTE_LOG(ERR, EAL, "Cannot find resource for device\n");
@@ -201,7 +203,8 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 	uint64_t pagesz;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct uio_resource *uio_res;
-	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
+	struct uio_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
 	struct uio_map *maps;
 
 	dev->intr_handle.fd = -1;
@@ -209,14 +212,16 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 
 	/* secondary processes - use already recorded details */
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return (pci_uio_map_secondary(dev));
+		return pci_uio_map_secondary(dev);
 
 	snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
 			dev->addr.bus, dev->addr.devid, dev->addr.function);
 
 	if (access(devname, O_RDWR) < 0) {
-		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
-				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
+		RTE_LOG(WARNING, EAL,
+			"  "PCI_PRI_FMT" not managed by UIO driver, "
+			"skipping\n", loc->domain, loc->bus,
+			loc->devid, loc->function);
 		return 1;
 	}
 
@@ -233,7 +238,7 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
 		RTE_LOG(ERR, EAL,
 			"%s(): cannot store uio mmap details\n", __func__);
-		return (-1);
+		return -1;
 	}
 
 	snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
@@ -248,7 +253,8 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 
 		j = uio_res->nb_maps;
 		/* skip empty BAR */
-		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
+		phaddr = dev->mem_resource[i].phys_addr;
+		if (phaddr == 0)
 			continue;
 
 		/* if matching map is found, then use it */
@@ -261,7 +267,7 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 						(size_t)maps[j].size)
 		    ) == NULL) {
 			rte_free(uio_res);
-			return (-1);
+			return -1;
 		}
 
 		maps[j].addr = mapaddr;
@@ -271,7 +277,7 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 
 	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
 
-	return (0);
+	return 0;
 }
 
 /* Scan one pci sysfs entry, and fill the devices list from it. */
@@ -311,7 +317,7 @@  pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	/* FreeBSD has no NUMA support (yet) */
 	dev->numa_node = 0;
 
-/* parse resources */
+	/* parse resources */
 	switch (conf->pc_hdr & PCIM_HDRTYPE) {
 	case PCIM_HDRTYPE_NORMAL:
 		max = PCIR_MAX_BAR_0;
@@ -440,32 +446,37 @@  rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 		/* check if device's identifiers match the driver's ones */
 		if (id_table->vendor_id != dev->id.vendor_id &&
-				id_table->vendor_id != PCI_ANY_ID)
+		    id_table->vendor_id != PCI_ANY_ID)
 			continue;
 		if (id_table->device_id != dev->id.device_id &&
-				id_table->device_id != PCI_ANY_ID)
+		    id_table->device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
+		if (id_table->subsystem_vendor_id !=
+		    dev->id.subsystem_vendor_id &&
+		    id_table->subsystem_vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
+		if (id_table->subsystem_device_id !=
+		    dev->id.subsystem_device_id &&
+		    id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
 
 		struct rte_pci_addr *loc = &dev->addr;
 
-		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				dev->numa_node);
+		RTE_LOG(DEBUG, EAL,
+			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			dev->numa_node);
 
-		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->name);
+		RTE_LOG(DEBUG, EAL,
+			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->name);
 
 		/* no initialization when blacklisted, return without error */
 		if (dev->devargs != NULL &&
 			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
 
-			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
+			RTE_LOG(DEBUG, EAL,
+				"  Device is blacklisted, not initializing\n");
 			return 0;
 		}
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 83c589e..353b0b8 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -154,7 +154,8 @@  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
 	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED | additional_flags, fd, offset);
 	if (mapaddr == MAP_FAILED) {
-		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot mmap(%d, %p, 0x%lx, 0x%lx): %s (%p)\n",
 			__func__, fd, requested_addr,
 			(unsigned long)size, (unsigned long)offset,
 			strerror(errno), mapaddr);
@@ -631,31 +632,36 @@  rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 		/* check if device's identifiers match the driver's ones */
 		if (id_table->vendor_id != dev->id.vendor_id &&
-				id_table->vendor_id != PCI_ANY_ID)
+		    id_table->vendor_id != PCI_ANY_ID)
 			continue;
 		if (id_table->device_id != dev->id.device_id &&
-				id_table->device_id != PCI_ANY_ID)
+		    id_table->device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
+		if (id_table->subsystem_vendor_id !=
+		    dev->id.subsystem_vendor_id &&
+		    id_table->subsystem_vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
+		if (id_table->subsystem_device_id !=
+		    dev->id.subsystem_device_id &&
+		    id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
 
 		struct rte_pci_addr *loc = &dev->addr;
 
-		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				dev->numa_node);
+		RTE_LOG(DEBUG, EAL,
+			"PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			dev->numa_node);
 
-		RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->name);
+		RTE_LOG(DEBUG, EAL,
+			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->name);
 
 		/* no initialization when blacklisted, return without error */
 		if (dev->devargs != NULL &&
 			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not initializing\n");
+			RTE_LOG(DEBUG, EAL,
+				"  Device is blacklisted, not initializing\n");
 			return 1;
 		}
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 2d1c69b..6f229d6 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -92,7 +92,8 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 {
 	int fd, i;
 	struct mapped_pci_resource *uio_res;
-	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
+	struct mapped_pci_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
 
@@ -117,14 +118,16 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 			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));
+						"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);
+						"Cannot mmap device resource "
+						"file %s to address: %p\n",
+						uio_res->maps[i].path,
+						uio_res->maps[i].addr);
 
 				close(fd);
 				return -1;
@@ -272,7 +275,8 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 	uint64_t phaddr;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
-	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
+	struct mapped_pci_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 	struct pci_map *maps;
 
 	dev->intr_handle.fd = -1;
@@ -286,8 +290,10 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 	/* find uio resource */
 	uio_num = pci_get_uio_dev(dev, dirname, sizeof(dirname));
 	if (uio_num < 0) {
-		RTE_LOG(WARNING, EAL, "  "PCI_PRI_FMT" not managed by UIO driver, "
-				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
+		RTE_LOG(WARNING, EAL,
+			"  "PCI_PRI_FMT" not managed by UIO driver, "
+			"skipping\n", loc->domain, loc->bus,
+			loc->devid, loc->function);
 		return 1;
 	}
 	snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
@@ -338,12 +344,11 @@  pci_uio_map_resource(struct rte_pci_device *dev)
 		if (phaddr == 0)
 			continue;
 
-
 		/* update devname for mmap  */
 		snprintf(devname, sizeof(devname),
 				SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/resource%d",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				i);
+				loc->domain, loc->bus, loc->devid,
+				loc->function, i);
 
 		/*
 		 * open resource file, to mmap it
@@ -412,7 +417,8 @@  static struct mapped_pci_resource *
 pci_uio_find_resource(struct rte_pci_device *dev)
 {
 	struct mapped_pci_resource *uio_res;
-	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
+	struct mapped_pci_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
 	if (dev == NULL)
 		return NULL;
@@ -431,7 +437,8 @@  void
 pci_uio_unmap_resource(struct rte_pci_device *dev)
 {
 	struct mapped_pci_resource *uio_res;
-	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
+	struct mapped_pci_res_list *uio_res_list =
+			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
 	if (dev == NULL)
 		return;