我对如何编写高质量的程序的看法

db101

我对如何编写高质量的程序的看法 [复制链接]

我对嵌入式软件开发的时间也不是很长，仅仅只有5年，算不上高手，也不是老手只能算是5岁的老菜鸟

，在前面的3年里没有高人指点，靠着自己摸索也写了不少的程序吧，那时侯我没有想过要写出容易理解
和维护以及移植的问题，仅仅想着如何把这个功能实现出来，所以在我工作的的第一个项目GSM汽车防盗

器，我用汇编写到5000行的时候逻辑就开始乱了，记得在快要完成所有功能的时候老板要我再增加一个

小功能时我不能决定在原有的程序的基础上哪个地方下手来增加，因为程序的逻辑关系太乱了，修改一

个地方会牵涉到很多功能，经过那一次以后我就下决心下次再写程序的时候一定要避免这个问题，一定

要作到容易修改，逻辑关系清晰。
后面有幸到一个外资公司做单片机软件开发工作，一个工程师给了我一本台湾出版的关于C语言面向

对象的编程的书（书名字忘记了),我在那本书里面找到了我为什么我第一个项目GSM汽车防盗器程序逻辑

乱的原因，我当时认认真真地读了那本书，真的感觉很好，在那本书里我学到了一个重要的概念---模块

化编程，加上那位台湾的工程师的指导，我觉得我的单片机编程水平有了质的飞跃，在以后的编程里我

不会再因为偷懒而不写注释，不会因为仅仅简单而放弃可维护，可移植，易懂的方案，这样慢慢养成了

习惯，在第二年我就用C写出了大约80KBYTE 的产品程序，而且自认为逻辑清晰，只要客户说需要修改哪

里我能马上作出响应。

  现在我已经习惯模块化程序设计，不管是8位的单片机还是32位的MPU，都会按照这样的思路去做，我

觉得写出好的，高质量的程序，
1.不要怕麻烦，该写的就要写，该做的就要做。
2.尽量模块化设计，虽然这样做在写小程序时会浪费时间，可这样会养成模块化程序的习惯。
3.最好多看看老外写的关于编程的书籍，你会发现很多东西你以前见到的。
4.学习下PSP,CMMI的课程.

下面是我用队列来实现CPU SCI 底层驱动程序的一个文件，希望能起到点点引用的作用。
最早是在freescale的CPU上使用，后来移植到了51,h8/3062,凌阳等MCU上，表现性能相当地好，在开发
产品节约了大把的时间，老板大大开心，我也大大开心。。
#define       SCI_DRIVER
/*****************************************************************************************
*Copyright: xxxxxCorporation

*
*File name: SciDriver.c

*
*Author: Kenny_wang

*
*Version: V1.0

*
*Date:                                                                      *
******************************************************************************************/

#include "SciDriver\SciDriver.h"
#include "SciDriver\Source\FunLst.h"
#include "SciDriver\Ports\Sciconfig.h"

/********************************************************************************
* Constant Define                               *
********************************************************************************/
#define       cQBufNormal          0
#define       cQBufFull          1
#define       cQBufEmpty          2

/********************************************************************************
* Queue structure                               *
********************************************************************************/
typedef struct{
      unsigned char    *pIn;
      unsigned char    *pOut;
      unsigned char    *pStart;
      unsigned int    bLength;
      unsigned int    wSize;
}QUEUE;

/********************************************************************************
* Sci structure                                  *
* Including tranmit and receive queue structure and Tx,Rx threshold variabls *
********************************************************************************/
typedef struct{
      unsigned char    bTxStatus;
      unsigned int    wTxLength;
      unsigned char    *pbTx;
      QUEUE *pqRx;
      unsigned char    bSciType;
}SciStruct;

/********************************************************************************
* List of Sci structure and queue                      *
********************************************************************************/
SciStruct SciList;
SciStruct *pSciIndex;
QUEUE QList;
unsigned char    bSciRxBuf[MAX_SCI_BUF_SIZE];
unsigned char    *pSciBuf = bSciRxBuf;
unsigned char    bSciNo = 0;

/********************************************************************************
* Internal Function Declaration                         *
********************************************************************************/
void sQInit(QUEUE *pq,unsigned char  *pStart,unsigned int  wSize);
unsigned char    sQDataIn(QUEUE *pq,unsigned char  bData);
unsigned char    sQDataOut(QUEUE *pq,unsigned char  *pData);

/********************************************************************************
*Function name: sQInit                               *
*Parameters: pq: pointer to queue structure to be initialized       *
*       start: start address of ring buffer             *
*       size: the size of the ring buffer             *
*Description: initialize a queue structure                   *
********************************************************************************/

db101

void sQInit(QUEUE *pq,unsigned char  *pStart,unsigned int  wSize)
{
pq->pIn = pStart;
pq->pOut = pStart;
pq->pStart = pStart;
pq->bLength = 0;
pq->wSize = wSize;
}

/********************************************************************************
*Function name: sQDataIn                         *
*Parameters: pq: pointer to queue structure to be initialized       *
*       data: the data to be inserted into the queue          *
*Returns: cQBufNormal: data has been inserted into the queue       *
*       cQBufFull: the buffer is full             *
*Description: insert a data into the queue                   *
********************************************************************************/
unsigned char    sQDataIn(QUEUE *pq,unsigned char  bData)
{
if(pq->bLength == pq->wSize)
{
      if(pq->pIn == pq->pStart)
      {
         *(pq->pStart + pq->wSize) = bData;
      }
      else
      {
         *(pq->pIn-1) = bData;
      }
      return(cQBufFull);
}
else
{
      *(pq->pIn) = bData;
      pq->bLength++;
      if(pq->pIn == pq->pStart + pq->wSize - 1)
      {
         pq->pIn = pq->pStart;
      }
      else
      {
         pq->pIn++;
      }
      return(cQBufNormal);
}
}

/********************************************************************************
*Function name: sQDataOut                         *
*Parameters: pq: pointer to queue structure to be initialized       *
*       pdata: the address to save the data             *
*Returns: cQBufNormal: data has been inserted into the queue       *
*       cQBufEmpty: the buffer is empty             *
*Description: Get a data from the queue                   *
********************************************************************************/
unsigned char    sQDataOut(QUEUE *pq,unsigned char  *pData)
{
if(pq->bLength == 0)
{
      return(cQBufEmpty);
}
*pData = *(pq->pOut);
pq->bLength--;
if(pq->pOut == pq->pStart + pq->wSize - 1)
{
      pq->pOut = pq->pStart;
}
else
{
      pq->pOut++;
}
return(cQBufNormal);
}

/********************************************************************************
*Function Name: sInitialSci                         *
*Parameters: bSciId:       sci id                      *
*       *bRxBuf: receive buffer start address          *
*       wRxSize: receive buffer length             *
*       bTxBuf:       transmit buffer start address          *
*       wTxSize: transmit buffer length             *
*       type:       sci type                   *
*Descriptions: assign and initialize the sci control struct to  sci       *
********************************************************************************/
void sInitialSci(unsigned int  wRxSize,unsigned char  bType)
{
QUEUE *pq;
SciStruct *pSci;

pSci = &SciList;
pSciIndex = pSci;

pSci->pqRx = &QList;
pq = pSci->pqRx;
sQInit(pq,pSciBuf,wRxSize);
pSciBuf += wRxSize;
bSciNo++;

pSci->bTxStatus = cSciTxRdy;
pSci->wTxLength = 0;

pSci->bSciType = bType;
}

/********************************************************************************
*Function Name: sSciRxISR                         *
*Parameters: bSciId:       sci id                      *
*Description: This function is executed in Sci rx interrupt io2sci rx compare *
*             interrupt.                   *
********************************************************************************/
void sSciRxISR(void)
{
unsigned char    bData;
QUEUE *pq;
SciStruct *pSci;

pSci = pSciIndex;
pq = pSci->pqRx;

if(sbGetSciRxRdy() == cSciRxRdy)
{
      sSciResetRx();
      bData = sbGetSciRxData();
      sQDataIn(pq,bData);
}
}

/********************************************************************************
*Function Name: sSciRead                         *
*Parameters: bSciId:       sci id                      *
*       *pBuf:       address to save data received          *
*Returns: cSciRxBufEmpty: receive  buffer is empty          *
*       cSciRxRdy: get one byte data successfully          *
*Description: This function is executed in AP                   *
********************************************************************************/
unsigned char    sSciRead(unsigned char  *pBuf)
{
QUEUE *pq;
unsigned char    bTemp;
SciStruct *pSci;

pSci = pSciIndex;
pq = pSci->pqRx;

OS_ENTER_CRITICAL();
bTemp = sQDataOut(pq,pBuf);
OS_EXIT_CRITICAL();

if(bTemp == cQBufEmpty)
{
      return(cSciRxBufEmpty);
}
else
{
      return(cSciRxRdy);
}
}

/********************************************************************************
*Function Name: sSciTxISR                         *
*Parameters: bSciId:       sci id                      *
*Description: This function is executed in Sci Tx interrupt io2sci Tx compare *
*             interrupt.                   *
********************************************************************************/
void sSciTxISR(void)
{
SciStruct *pSci;

pSci = pSciIndex;

if(sbGetSciTxRdy() == cSciTxRdy)
{
      if(pSci->wTxLength == 0)
      {
         pSci->bTxStatus = cSciTxRdy;
         sSciResetTx();
      }
      else
      {
         sSciTxData(*(pSci->pbTx));
         (pSci->pbTx)++;
         (pSci->wTxLength)--;
         sSciResetTx();
      }
}
}

/********************************************************************************
*Function Name: sSciWrite                         *
*Parameters: bSciId:       sci id                      *
*       *pstart: start address of data to be sent       *
*       wLength: the length of data to be send          *
*Returns: cSciTxBufFull: transmit  buffer is empty          *
*       cSciTxRdy: send one byte data successfully          *
*Description: This function is executed in AP                   *
********************************************************************************/

db101

unsigned char    sSciWrite(unsigned char  *pStart,unsigned int  wLength)
{
SciStruct *pSci;

pSci = pSciIndex;

if(pSci->bTxStatus == cSciTxBusy)
{
      return(cSciTxBusy);
}

OS_ENTER_CRITICAL();
pSci->pbTx = pStart;
pSci->wTxLength = wLength;
pSci->bTxStatus = cSciTxBusy;

sSciTxData(*(pSci->pbTx));
(pSci->pbTx)++;
(pSci->wTxLength)--;

OS_EXIT_CRITICAL();

return(cSciTxRdy);
}

/********************************************************************************
*Function Name: sbGetSciTxStatus                      *
*Parameters: bSciId:          sci id                   *
*Returns: sci tx status       cSciTxRdy             *
*                   cSciTxBusy             *
*Description: Get the sci trasmit status                   *
********************************************************************************/
unsigned char    sbGetSciTxStatus(void)
{
SciStruct *pSci;

pSci = pSciIndex;

return(pSci->bTxStatus);
}

/********************************************************************************
*Function Name: sSetSciBaudRate                         *
*Parameters: bSciId:          sci id                   *
*Returns: bBaudrate       Sci Baudrate             *
*Description: Set the sci baudrate                      *
********************************************************************************/
void sSetSciBaudRate(unsigned char  bBaudrate)
{
sSciChangeBaudRate(bBaudrate);
}

zheng_ji_yi

大体看了看代码，改进的地方很大，需要更加努力！建议多看看人家的代码。附上linux内核实现的fifo。
kfifo.c
/*
* A simple kernel FIFO implementation.
*
* Copyright (C) 2004 Stelian Pop <stelian@popies.net>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*
*/

#include
#include
#include
#include
#include

/**
* kfifo_init - allocates a new FIFO using a preallocated buffer
* @buffer: the preallocated buffer to be used.
* @size: the size of the internal buffer, this have to be a power of 2.
* @gfp_mask: get_free_pages mask, passed to kmalloc()
* @lock: the lock to be used to protect the fifo buffer
*
* Do NOT pass the kfifo to kfifo_free() after use ! Simply free the
* struct kfifo with kfree().
*/
struct kfifo *kfifo_init(unsigned char *buffer, unsigned int size,
         gfp_t gfp_mask, spinlock_t *lock)
{
struct kfifo *fifo;

/* size must be a power of 2 */
BUG_ON(size & (size - 1));

fifo = kmalloc(sizeof(struct kfifo), gfp_mask);
if (!fifo)
      return ERR_PTR(-ENOMEM);

fifo->buffer = buffer;
fifo->size = size;
fifo->in = fifo->out = 0;
fifo->lock = lock;

return fifo;
}

zheng_ji_yi

EXPORT_SYMBOL(kfifo_init);

/**
* kfifo_alloc - allocates a new FIFO and its internal buffer
* @size: the size of the internal buffer to be allocated.
* @gfp_mask: get_free_pages mask, passed to kmalloc()
* @lock: the lock to be used to protect the fifo buffer
*
* The size will be rounded-up to a power of 2.
*/
struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask, spinlock_t *lock)
{
unsigned char *buffer;
struct kfifo *ret;

/*
   * round up to the next power of 2, since our 'let the indices
   * wrap' tachnique works only in this case.
   */
if (size & (size - 1)) {
      BUG_ON(size > 0x80000000);
      size = roundup_pow_of_two(size);
}

buffer = kmalloc(size, gfp_mask);
if (!buffer)
      return ERR_PTR(-ENOMEM);

ret = kfifo_init(buffer, size, gfp_mask, lock);

if (IS_ERR(ret))
      kfree(buffer);

return ret;
}
EXPORT_SYMBOL(kfifo_alloc);

/**
* kfifo_free - frees the FIFO
* @fifo: the fifo to be freed.
*/
void kfifo_free(struct kfifo *fifo)
{
kfree(fifo->buffer);
kfree(fifo);
}
EXPORT_SYMBOL(kfifo_free);

/**
* __kfifo_put - puts some data into the FIFO, no locking version
* @fifo: the fifo to be used.
* @buffer: the data to be added.
* @len: the length of the data to be added.
*
* This function copies at most 'len' bytes from the 'buffer' into
* the FIFO depending on the free space, and returns the number of
* bytes copied.
*
* Note that with only one concurrent reader and one concurrent
* writer, you don't need extra locking to use these functions.
*/
unsigned int __kfifo_put(struct kfifo *fifo,
         unsigned char *buffer, unsigned int len)
{
unsigned int l;

len = min(len, fifo->size - fifo->in + fifo->out);

/* first put the data starting from fifo->in to buffer end */
l = min(len, fifo->size - (fifo->in & (fifo->size - 1)));
memcpy(fifo->buffer + (fifo->in & (fifo->size - 1)), buffer, l);

/* then put the rest (if any) at the beginning of the buffer */
memcpy(fifo->buffer, buffer + l, len - l);

fifo->in += len;

return len;
}
EXPORT_SYMBOL(__kfifo_put);

zheng_ji_yi

/**
* __kfifo_get - gets some data from the FIFO, no locking version
* @fifo: the fifo to be used.
* @buffer: where the data must be copied.
* @len: the size of the destination buffer.
*
* This function copies at most 'len' bytes from the FIFO into the
* 'buffer' and returns the number of copied bytes.
*
* Note that with only one concurrent reader and one concurrent
* writer, you don't need extra locking to use these functions.
*/
unsigned int __kfifo_get(struct kfifo *fifo,
         unsigned char *buffer, unsigned int len)
{
unsigned int l;

len = min(len, fifo->in - fifo->out);

/* first get the data from fifo->out until the end of the buffer */
l = min(len, fifo->size - (fifo->out & (fifo->size - 1)));
memcpy(buffer, fifo->buffer + (fifo->out & (fifo->size - 1)), l);

/* then get the rest (if any) from the beginning of the buffer */
memcpy(buffer + l, fifo->buffer, len - l);

fifo->out += len;

return len;
}
EXPORT_SYMBOL(__kfifo_get);

头文件kfifo.h
/*
* A simple kernel FIFO implementation.
*
* Copyright (C) 2004 Stelian Pop <stelian@popies.net>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*
*/
#ifndef _LINUX_KFIFO_H
#define _LINUX_KFIFO_H

#ifdef __KERNEL__

#include
#include

struct kfifo {
unsigned char *buffer; /* the buffer holding the data */
unsigned int size; /* the size of the allocated buffer */
unsigned int in; /* data is added at offset (in % size) */
unsigned int out; /* data is extracted from off. (out % size) */
spinlock_t *lock; /* protects concurrent modifications */
};

extern struct kfifo *kfifo_init(unsigned char *buffer, unsigned int size,
            gfp_t gfp_mask, spinlock_t *lock);
extern struct kfifo *kfifo_alloc(unsigned int size, gfp_t gfp_mask,
               spinlock_t *lock);
extern void kfifo_free(struct kfifo *fifo);
extern unsigned int __kfifo_put(struct kfifo *fifo,
            unsigned char *buffer, unsigned int len);
extern unsigned int __kfifo_get(struct kfifo *fifo,
            unsigned char *buffer, unsigned int len);

/**
* __kfifo_reset - removes the entire FIFO contents, no locking version
* @fifo: the fifo to be emptied.
*/
static inline void __kfifo_reset(struct kfifo *fifo)
{
fifo->in = fifo->out = 0;
}

/**
* kfifo_reset - removes the entire FIFO contents
* @fifo: the fifo to be emptied.
*/
static inline void kfifo_reset(struct kfifo *fifo)
{
unsigned long flags;

spin_lock_irqsave(fifo->lock, flags);

__kfifo_reset(fifo);

spin_unlock_irqrestore(fifo->lock, flags);
}

/**
* kfifo_put - puts some data into the FIFO
* @fifo: the fifo to be used.
* @buffer: the data to be added.
* @len: the length of the data to be added.
*
* This function copies at most 'len' bytes from the 'buffer' into
* the FIFO depending on the free space, and returns the number of
* bytes copied.
*/
static inline unsigned int kfifo_put(struct kfifo *fifo,
                  unsigned char *buffer, unsigned int len)
{
unsigned long flags;
unsigned int ret;

spin_lock_irqsave(fifo->lock, flags);

ret = __kfifo_put(fifo, buffer, len);

spin_unlock_irqrestore(fifo->lock, flags);

return ret;
}

/**
* kfifo_get - gets some data from the FIFO
* @fifo: the fifo to be used.
* @buffer: where the data must be copied.
* @len: the size of the destination buffer.
*
* This function copies at most 'len' bytes from the FIFO into the
* 'buffer' and returns the number of copied bytes.
*/
static inline unsigned int kfifo_get(struct kfifo *fifo,
                  unsigned char *buffer, unsigned int len)
{
unsigned long flags;
unsigned int ret;

spin_lock_irqsave(fifo->lock, flags);

ret = __kfifo_get(fifo, buffer, len);

/*
   * optimization: if the FIFO is empty, set the indices to 0
   * so we don't wrap the next time
   */
if (fifo->in == fifo->out)
      fifo->in = fifo->out = 0;

spin_unlock_irqrestore(fifo->lock, flags);

return ret;
}

/**
* __kfifo_len - returns the number of bytes available in the FIFO, no locking version
* @fifo: the fifo to be used.
*/
static inline unsigned int __kfifo_len(struct kfifo *fifo)
{
return fifo->in - fifo->out;
}

/**
* kfifo_len - returns the number of bytes available in the FIFO
* @fifo: the fifo to be used.
*/
static inline unsigned int kfifo_len(struct kfifo *fifo)
{
unsigned long flags;
unsigned int ret;

spin_lock_irqsave(fifo->lock, flags);

ret = __kfifo_len(fifo);

spin_unlock_irqrestore(fifo->lock, flags);

return ret;
}

#else
#warning "don't include kernel headers in userspace"
#endif /* __KERNEL__ */
#endif

jason022

虽然有人指出不是，但是感觉还是不错得，给你裤子了。

db101

我的目的只是在一般MCU上实现，上边的文件和另外一个文件配合就不需要修改任何地方，我会继续努力的。

我对如何编写高质量的程序的看法 [复制链接]

最新回复