[apue] 书中关于打印服务代码的一个错误

[apue] 书中关于打印服务代码的一个错误

在看 apue 第 21 章 与网络打印机通信一章时，发现一段关于链表操作的代码有问题，现在摘出来让大家 review 一下。先上代码：

printd.c

 

这是打印服务的源代码，在打印时，用户通过  print 命令提交待打印的文件，print 命令通过 tcp 与 printd 服务通讯，

将文件及打印相关的参数传递给后者；对于每个客户，printd 服务会创建一个 worker 结构节点，

放在一个由 workers 变量指定了头的双向链表中。所以这段代码本质上就是简单的双向链接操作：

 1 void add_worker (pthread_t tid, int sockfd) 2 { 3  struct worker_thread *wtp;  4  if ((wtp = malloc (sizeof (struct worker_thread))) == NULL) {  5   log_ret ("add_worker: can't malloc");  6   pthread_exit ((void *)1);  7  } 8  9  wtp->tid = tid; 10  wtp->sockfd = sockfd; 11 12  log_msg ("prepare to add worker"); 13  pthread_mutex_lock (&workerlock); 14 15  wtp->prev = NULL; 16  wtp->next = workers; 17  if (workers == NULL)18   workers = wtp; 19  else 20   workers->prev = wtp; 21 22  pthread_mutex_unlock (&workerlock); 23 }

 

重点就是 15-20 这 6 行啦，当第一次加入节点时， workers 为 NULL，所以走第一个条件分支，这没有问题；

但是再加入节点时， workers 不为 NULL，此时走 else 分支，将当前头的上一个节点设置为待插入的新节点  wtp，

到现在还好，可是等等，怎么就没下文了？！这个节点还没完全加入链表呢……

正确的做法应该是在结尾前再加一句：

 else  {  workers->prev = wtp;   workers = wtp;  }

 

这样才能算完嘛。道理就不多说了，不信自己画个链表看看。下面给出优化后的完整代码：

 1 void add_worker (pthread_t tid, int sockfd) 2 { 3  struct worker_thread *wtp;  4  if ((wtp = malloc (sizeof (struct worker_thread))) == NULL) {  5   log_ret ("add_worker: can't malloc");  6   pthread_exit ((void *)1);  7  } 8  9  wtp->tid = tid; 10  wtp->sockfd = sockfd; 11  pthread_mutex_lock (&workerlock); 12 13  wtp->prev = NULL; 14  wtp->next = workers; 15  if (workers != NULL)16   workers->prev = wtp; 17 18  workers = wtp; 19 20  pthread_mutex_unlock (&workerlock); 21 }

 

好吧，我承认作为经典著作也会有这种低级错误。

今天的吹毛求疵就到这里，作为一个有职业素养的程序员，不在鸡蛋里挑出骨头来不罢休，嘿嘿……