函数
- 函数应该简短而漂亮,并且只完成一件事情。
- 一个函数的最大长度是和该函数的复杂度和缩进级数成反比的。
比如一个函数仅是一个很长的(但很简单的)switch语句, 有很多个case,而且在每个case里都是做很小的事情,这样的函数尽管很长,但也是可以的。
不过,如果你有一个复杂的函数,而且你怀疑一个天分不是很高的高中一年级学生可能甚至搞不清楚这个函数的目的,你应该严格遵守前面提到的长度限制。
使用辅助函数,并为之取个具描述性的名字(如果你觉得它们的性能很重要的话,可以让编译器内联它们,这样的效果往往会比你写一个复杂函数的效果要好) - 函数的另外一个衡量标准是本地变量的数量。此数量不应超过 5-10 个,否则你的函数 就有问题了。
重新考虑一下你的函数,把它分拆成更小的函数。人的大脑一般可以轻松的同时跟踪7个不同的事物,如果再增多的话,就会糊涂了。
即便你聪颖过人,你也可能会记不清你2个星期前做过的事情。
缩进
如果你需要3级以上的缩进,不管用何种方式你的代码已经有问题了,应该修正你的程序
下面是一个减少缩进的例子:
> + if (!(hctx->flags & BLK_MQ_F_FORCE_COMMIT_RQS)) {
> + if (list_empty(list)) {
> + bd.last = true;
> + } else {
> + nxt = list_first_entry(list, struct request,
> + queuelist);
> + bd.last = !blk_mq_get_driver_tag(nxt);
> + }
> + } else {
> + bd.last = false;
> }
改成下面这样, 逻辑是不是更清晰:
if (hctx->flags & BLK_MQ_F_FORCE_COMMIT_RQS) {
bd.last = false;
} else if (list_empty(list)) {
bd.last = true;
} else {
nxt = list_first_entry(list, struct request, queuelist);
bd.last = !blk_mq_get_driver_tag(nxt);
}
不要用typedef
对结构体和指针使用 typedef 是一个错误 。当你在代码里看到:
vps_t a;
这代表什么意思呢? 相反,如果是这样
struct virtual_container *a;
你就知道 a 是什么了。
总的来说,如果一个指针或者一个结构体里的元素可以合理的被直接访问到,那么它们就不应该是一个typedef。
使用typedef另一个坏处是容易导致重名:
/space/builder/repo/build_4_12987/code/bsp/modules/gpu/gondul/mali/memory_group_manager.h:31:13: error: typedef redefinition with different types (‘int’ vs ‘unsigned int’)
typedef int vm_fault_t;
^
/space/builder/repo/build_4_12987/code/bsp/kernel/kernel4.14/include/linux/mm.h:1267:32: note: previous definition is here
typedef __bitwise unsigned int vm_fault_t;
有人可能会说, 这种问题在编译阶段就报出来了, 但请注意不同的config导致编译的范围不同, 有些编译错误容易被遗漏, 所以在编码层面避免最安全.
宏,枚举
使用宏的时候应避免的事情:
影响控制流程的宏:
#define FOO(x) \
do { \
if (blah(x) < 0) \
return -EBUGGERED; \
} while (0)
非常不好。它看起来像一个函数,不过却能导致调用它的函数退出.
使用表达式定义常量的宏必须将表达式置于一对小括号之内。带参数的宏也要注意此类问题。
#define CONSTANT 0x4000
#define CONSTEXP (CONSTANT | 3)
在宏内部定义变量时注意避免命名冲突:
#define FOO(x) \
({ \
typeof(x) ret; \
ret = calc_ret(x); \
(ret); \
})
ret 是本地变量的通用名字, __foo_ret 更不容易与一个已存在的变量冲突。
分配内存
- 传递结构体大小的首选形式是这样的:p = kmalloc(sizeof(*p), …);
另外一种传递方式中,sizeof 的操作数是结构体的名字,这样会降低可读性,并且可能会引入bug.
有可能指针变量类型被改变时,而对应的传递给内存分配函数的 sizeof 的结果不变. - 强制转换一个 void 指针返回值是多余的。C语言本身保证了从 void 指针到其他任何指针类型的转换是没有问题的.
- 内存分配失败要返回-ENOMEM, 但无需增加error log, mm子系统会自动输出信息.
- 分配一个数组的首选形式是:p = kmalloc_array(n, sizeof(…), …);
- kmemdup()函数是根据给定的一段地址区间,再分配一个内存空间,将原地址空间的内容拷贝到新分配的内存空间中.
- char *kstrdup(const char *s, gfp_t gfp) 是为常量字符串s分配内存空间,并将该字符串拷贝到所分配的地址空间中.
- 申请的内存用完一定要记得free (devm 接口分配的除外)
- 用devm_kmalloc()分配的内存,当device被detached或者驱动卸载时或者driver初始化失败,内存会被自动释放, 所以无需显示释放(尤其不能用kfree). 只有当正常运行态且内存不再使用时,可以使用函数devm_kfree()释放
命名 – 给命名内涵
好命名要做到见名知意。
a) 变量名 – 给变量意义
变量名应该是个名词,或是名词属性的词组,如果有计量单位的应该写上单位。
比如一个shit 命名如下:
int d; // 表示过去的天数
为什么不把写注释的时间,用来起一个有意义的名字呢?? 比如:
int elapsed_time_day;
int elapsed_time_hour;
int elapsed_time_minute;
变量名不要用有多个含义的词
比如number,既有“数字” 又有 “号码”的意思,用来命名会产生歧义,有时需要阅读整段代码才能知道是什么含义。
可以尝试其他的名词,比如:
count:数目
id:号码
想也想不到好名字,看看别人的是怎么用的吧。
命名查询工具:
在线版: https://unbug.github.io/codelf/
插件版: https://github.com/unbug/codelf
b) 函数名 – 给函数目的
函数名应该是描述一个动作或者过程的目的,而不是一个做法。
比如一个不好的命名, 通过函数名并不知道这个函数要干吗:
employee process_employee_data();
可以通过业务的逻辑,写成如下函数 - 好的函数名要描述一个目的:
employee get_lastest_employee();
具有相反功能的函数命名要对仗, 比如 file_open 和 f_close,就不对仗,应改写成:
file_open() / file_close()
file_create() / file_delete()
c) 代码中一般不应该出现hard code,常数应该有一个有意义的名字:
#define SECONDS_PER_DAY (86400)
int seconds_per_day = SECONDS_PER_DAY;
条件编译
只要可能,就不要在 .c 文件里面使用预处理条件 (#if, #ifdef);这样做让代码更难阅读并且更难去跟踪逻辑。
替代方案是:
- 在头文件中用预处理条件提供给那些.c 文件使用
- 再给#else 提供一个空的桩函数(no-op stub)
- 然后在.c文件内无条件地调用那些(定义在头文件内的)函数
这样做,编译器会避免为桩函数(stub)的调用生成任何代码,产生的结果是相同的,但逻辑将更加清晰。
空行
合理的使用空行,可以增加代码或文章的可读性。
一个可行的参考原则是:
将概念相关的代码放在一起,相关性越强,彼此之间的距离就越短。
函数返回值
- 一般返回0表示成功,负值表示错误
- 使用kernel定义的标准错误码作为错误返回值,不要自己随意定义错误码,每种错误场景都返回-1是不明智的
- 不同的错误场景使用不同的错误码, 比如:
-ENOMEM: 内存分配失败
-EINVAL: 无效的参数
-EIO: IO错误
-ETIMEOUT: 超时错误 - 一般不要覆盖返回值,尤其是EPROBE_DEFER
- 一般情况下,有返回值的函数需要检查返回值, 并进行相应处理
参考资料:
https://www.kernel.org/doc/html/latest/process/coding-style.html
鲁棒性:硬代码
- 访问firmware分区/文件路径不应假设分区/目录一定存在
容错设计,应尝试多个可能路径,判断返回值,并进行处理(报错)。
From: 耿慈熙 (Cixi Geng/10039)
Sent: Wednesday, July 8, 2020 14:52
To: 张春艳 (Chunyan Zhang)
Subject: 转发: 【WCN的AP侧UT&IT试用跟踪】AP侧Kernel部分的UT测试框架使用KUNIT, 具体请参考系列SVN的培训文档进行,有问题请找【耿慈熙】技术支持,大家使用过程有任何问题都及时反馈,谢谢。
Hi chunyan:
bt驱动加载的时候报错为"ERROR:try to open file /dev/block/platform-soc/soc:ap-abb/20600000.sdio/by-name/***
此设计依赖android设计
[鲁棒性:超时处理:]
11. 超时处理原则
例1:
Mailbox: http://review.source.unisoc.com/gerrit/#/c/690573/
时延的方法
1)忙等:
直接调用系统通用的mdelay或udelay,但是他们的精度不高。
26 void __delay(unsigned long cycles)
27 {
28 cycles_t start = get_cycles();
29
30 if (arch_timer_evtstrm_available()) {
31 const cycles_t timer_evt_period =
32 USECS_TO_CYCLES(ARCH_TIMER_EVT_STREAM_PERIOD_US);
33
34 while ((get_cycles() - start + timer_evt_period) < cycles)
35 wfe();
36 }
37
38 while ((get_cycles() - start) < cycles)
39 cpu_relax();
40 }
41 EXPORT_SYMBOL(__delay);
也是使用jiffies + 空循环。
2)睡眠等:
使用精确定时hrtimer函数。
一个比较好的寄存器等待bit位的例子如下:
488 static inline bool wait_for_bit_change(struct dsi_data *dsi,
489 const struct dsi_reg idx,
490 int bitnum, int value)
491 {
492 unsigned long timeout;
493 ktime_t wait;
494 int t;
495
496 /* first busyloop to see if the bit changes right away */
497 t = 100;
498 while (t-- > 0) {
499 if (REG_GET(dsi, idx, bitnum, bitnum) == value)
500 return true;
501 }
502
503 /* then loop for 500ms, sleeping for 1ms in between */
504 timeout = jiffies + msecs_to_jiffies(500);
505 while (time_before(jiffies, timeout)) {
506 if (REG_GET(dsi, idx, bitnum, bitnum) == value)
507 return true;
508
509 wait = ns_to_ktime(1000 * 1000);
510 set_current_state(TASK_UNINTERRUPTIBLE); //睡眠1ms,让出cpu.
511 schedule_hrtimeout(&wait, HRTIMER_MODE_REL);
512 }
513
514 return false;
515 }
3) 简易空循环:无法保证时间精度,不推荐。
如:
for (i=0; i<1000;i++) {
if (read(bit) == 1)
break;
cpu_relax();
}
4) 死循环: 应该避免!
如:
while (read(bit)==1)
cpu_relax();
[鲁棒性:DT解析:]
13. DTS解析不能依赖顺序
发件人: 叶景超 (Jingchao Ye) 发送时间: 2020年7月7日 20:25
主题: RE: overlay dts节点无法正常获取 sharkl3 dvfs failed 问题攻关 需求 Hi, All,
与weidong确认下来,目前只要定义在 sp9863a-3h10-overlay.dts 中的所有节点,其子节点的 顺序 会变成 逆序,
即 dvfs的驱动 是顺序parse 前3个 子节点 A/B/C,逆序之后变成parse 子节点E/D/C,导致逻辑出错
如果有其它的驱动也是依赖于这个顺序parse,也会遇到同样的问题。 …
[鲁棒性:others:]
14. 不能随意使用BUG_ON
除非是程序内部已经被损害导致系统无论如何都会死, 其他情况通通不允许使用BUG_ON().
取而代之, 应该这样做:
if (WARN_ON_ONCE(…))
return -EINVAL;
Linus Torvalds曾多次解释过这个问题:
https://lkml.org/lkml/2015/6/24/662
https://lkml.org/lkml/2015/6/7/193
15. 遵循内核控制流完整性(KCFI)要求
通过不兼容的函数指针调用 indirect 函数,将导致 CFI 故障。当检测到 CFI 故障时,内核会输出一条警告(或者panic),其中包括被调用的函数和导致故障的堆栈轨迹。您可以通过确保函数指针始终与调用的函数属于同一类型来修正此问题。
看下面这个例子就明白了:
struct cam_hw_info {
...
int (*dcam_ioctl)(void *,uint32_t, void *);
};
dcam_ioctl是一个函数指针, 它的其中一个实体定义如下:
static int sharkl5pro_dcam_ioctl(void *handle, enum dcam_hw_cfg_cmd cmd, void *arg)
可以注意到dcam_ioctl定义的第二个参数类型是uint32_t, 而实体函数sharkl5pro_dcam_ioctl的第二个参数类型是enum dcam_hw_cfg_cmd cmd, 两个参数类型不一致, 将导致每一次通过dcam_ioctl函数指针进行间接调用的时候, CFI就会告警或者直接panic.
CFI 文档说明: https://source.android.com/devices/tech/debug/kcfi
- 借助外部控制信号进行通断开关的DT及程序方法
1). 外接信号为regulator引出:
DT写法举例:
vref-supply = <&adc_vref>;
驱动处理方式:
241 st->reg = devm_regulator_get(&spi->dev, "vref");
242 if (IS_ERR(st->reg))
243 return PTR_ERR(st->reg);
244
245 ret = regulator_enable(st->reg);
246 if (ret < 0) {
247 dev_err(&spi->dev, "Cannot enable vref regulator\n");
248 return ret;
249 }
250
251 ret = regulator_get_voltage(st->reg);
252 if (ret < 0) {
253 dev_err(&spi->dev, "vref get voltage failed\n");
254 return ret;
255 }
256
2). 外接信号为GPIO引出:
DT写法举例:
enable-gpios = <&gpio 27 0>;
驱动处理方式:
906 pdata->enable_gpio = of_get_named_gpio(np, "enable-gpios", 0);
...
768 gpio = lp->pdata->enable_gpio;
769 if (!gpio_is_valid(gpio))
770 return 0;
771
772 /* Always set enable GPIO high. */
773 ret = devm_gpio_request_one(lp->dev, gpio, GPIOF_OUT_INIT_HIGH, "LP872X EN");
以上代码示例均取自kernel 4.14