On Thu, May 18, 2017 at 12:14:29PM -0700, Jork Loeser wrote:
> static int hv_pci_protocol_negotiation(struct hv_device *hdev)
> {
> + size_t i;
Could you just use "int i". I know some static checkers complain but
those tools are dumb. I just fixed a couple bugs two days ago where
people were like, "If i is declared as a u32 that means it's safe" and
it turns out, nope. No need to get fancy.
And could you put the i at the end of the declaration block in reverse
Christmas tree order? It matches the others in this function.
loooooooooooooooooooooooooooong var;
meeeeeeedium var;
int ret;
int i;
> struct pci_version_request *version_req;
> struct hv_pci_compl comp_pkt;
> struct pci_packet *pkt;
> @@ -1816,28 +1832,44 @@ static int hv_pci_protocol_negotiation(struct hv_device *hdev)
> pkt->compl_ctxt = &comp_pkt;
> version_req = (struct pci_version_request *)&pkt->message;
> version_req->message_type.type = PCI_QUERY_PROTOCOL_VERSION;
> - version_req->protocol_version = PCI_PROTOCOL_VERSION_CURRENT;
>
> - ret = vmbus_sendpacket(hdev->channel, version_req,
> - sizeof(struct pci_version_request),
> - (unsigned long)pkt, VM_PKT_DATA_INBAND,
> - VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> - if (ret)
> - goto exit;
> + for (i = 0; i < ARRAY_SIZE(pci_protocol_versions); i++) {
> + version_req->protocol_version = pci_protocol_versions[i];
> + ret = vmbus_sendpacket(
> + hdev->channel, version_req,
> + sizeof(struct pci_version_request),
> + (unsigned long)pkt, VM_PKT_DATA_INBAND,
> + VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
The indenting is messed up because VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED
is really long. http://new_words.enacademic.com/2023/noun-banging
NOUN_NOUN_NOUN_NOUN_NOUN_ADJECTIVE. I guess do this:
ret = vmbus_sendpacket(hdev->channel, version_req,
sizeof(*version_req), (unsigned long)pkt,
VM_PKT_DATA_INBAND,
VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> + if (ret)
> + goto exit;
This "goto exit;" prints a successful message, but it's a failure path.
We also print a message on every iteration through this function. Since
we only go through the function once in the current code it's works but
let's fix it.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
