Over 80-character line tips

Breaking comment lines

When breaking up comment lines that are over 80-characters, make sure to use the correct multi-line comment style, which is either:

        /* One line.
         * Two lines.
         */

or

        /*
         * One line
         * Two lines.
         */

Different driver authors use different styles, so use the style that is most common in the driver.

Breaking function calls

Sometimes a call to a function has several variables, and you need to break the line in the middle of those variables. Look at this example:

        pdata->urbdata = usb_alloc_coherent(pdata->udev, ACD_URB_BUFFER_LEN, GFP_KERNEL, &pdata->urb->transfer_dma);

This line is too long, so we want to break it up. By default, vim will increase the indentation of the trailing line by one tab:

        pdata->urbdata = usb_alloc_coherent(pdata->udev, ACD_URB_BUFFER_LEN,
                GFP_KERNEL, &pdata->urb->transfer_dma);

This style is fine, and generally perferred. However, some driver writers prefer to have the trailing line of a function call line up with the starting '('. They use tabs, followed by spaces, to align the trailing line:

        pdata->urbdata = usb_alloc_coherent(pdata->udev, ACD_URB_BUFFER_LEN,
                                            GFP_KERNEL,
                                            &pdata->urb->transfer_dma);

Again, the one tab indent style is preferred, but don't change lines that use the other style.

Breaking long math lines

When breaking up long math lines, make sure to break after an arthimetic or boolean operator, like this:

        really_long_function_call(other_long_function_call(variable) &&
                        yet_another_long_function_call(variable));

Breaking with dereferences or unary operators

Do not break lines at the dereference operator (->), even if it means leaving a very long line in.

Likewise, do not break a line between a unary operator (such as ! or *) and its argument.

Parens in return value

Ignore checkpatch.pl if it complains about parens around boolean expressions or ternary conditionals in return values, like this:

        return ((depth > 1) ? (depth - 1) : depth);

sscanf

When changing code to check the return value of sscanf, make sure that you properly clean up before returning. Take, for example, this code in gdm_lte_tx() in drivers/staging/gdm724x/gdm_lte.c:

        if (nic_type & NIC_TYPE_ICMPV6) {
                if (gdm_lte_emulate_ndp(skb, nic_type) == 0) {
                        dev_kfree_skb(skb);
                        return 0;
                }
        }

        /*
        Need byte shift (that is, remove VLAN tag) if there is one
        For the case of ARP, this breaks the offset as vlan_ethhdr+4 is treated as ethhdr
        However, it shouldn't be a problem as the response starts from arp_hdr and ethhdr
        is created by this driver based on the NIC mac
        */
        if (nic_type & NIC_TYPE_F_VLAN) {
                struct vlan_ethhdr *vlan_eth = (struct vlan_ethhdr *)skb->data;
                nic->vlan_id = ntohs(vlan_eth->h_vlan_TCI) & VLAN_VID_MASK;
                data_buf = skb->data + (VLAN_ETH_HLEN - ETH_HLEN);
                data_len = skb->len - (VLAN_ETH_HLEN - ETH_HLEN);
        } else {
                nic->vlan_id = 0;
                data_buf = skb->data;
                data_len = skb->len;
        }

        /* If it is a ICMPV6 packet, clear all the other bits : for backward compatibility with the firmware */
        if (nic_type & NIC_TYPE_ICMPV6)
                nic_type = NIC_TYPE_ICMPV6;

        /* If it is not a dhcp packet, clear all the flag bits : original NIC, otherwise the special flag (IPVX | DHCP) */
        if (!(nic_type & NIC_TYPE_F_DHCP))
                nic_type &= NIC_TYPE_MASK;

        sscanf(dev->name, "lte%d", &idx);

When trying to fix the code to check the return value of sscanf, you may be tempted to simply write this patch:

@@ -444,7 +444,9 @@ static int gdm_lte_tx(struct sk_buff *skb, struct net_device *dev)
        if (!(nic_type & NIC_TYPE_F_DHCP))
                nic_type &= NIC_TYPE_MASK;

-       sscanf(dev->name, "lte%d", &idx);
+       ret = sscanf(dev->name, "lte%d", &idx);
+       if (ret != 1)
+               return -EINVAL;

        ret = nic->phy_dev->send_sdu_func(nic->phy_dev->priv_dev,
                                          data_buf, data_len,

However, this patch will cause a bug because it leaks memory. If you look carefully at the code before the sscanf call for return statements, you'll notice the code frees some memory before it returns:

        if (nic_type & NIC_TYPE_ICMPV6) {
                if (gdm_lte_emulate_ndp(skb, nic_type) == 0) {
                        dev_kfree_skb(skb);
                        return 0;
                }
        }

Therefore, your patch should also free this memory before it returns:

@@ -444,7 +444,9 @@ static int gdm_lte_tx(struct sk_buff *skb, struct net_device *dev)
        if (!(nic_type & NIC_TYPE_F_DHCP))
                nic_type &= NIC_TYPE_MASK;

-       sscanf(dev->name, "lte%d", &idx);
+       ret = sscanf(dev->name, "lte%d", &idx);
+       if (ret != 1) {
+               dev_kfree_skb(skb);
+               return -EINVAL;
+       }
        ret = nic->phy_dev->send_sdu_func(nic->phy_dev->priv_dev,
                                          data_buf, data_len,

You'll need to look very carefully at the code in between the last return statement and the sscanf to make sure further tear down is not needed. Sometimes looking at the tear-down in return statements after the sscanf may help.