Re: [LTP] [PATCH v2] pipeio/pipeio.c: cleanup

[Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>]

Hi,
On 04/10/2014 07:27 PM, Jan Stancek wrote:
>
>> +static int Nchildcomplete;
> I'd suggest sig_atomic_t here.

Yes, it should be.
>
>> +
>> +static int error;
>> +static int count;
>> +static int num_writers = 1;	/* number of writers */
>> +static int num_writes = 1;	/* number of writes per child */
>> +static int loop;		/* loop indefinitely */
>> +static int exit_error = 1;	/* exit on error #, zero means no exit */
>> +static int size = 327;		/* default size */
>> +static int unpipe;		/* un-named pipe if non-zero */
>> +static int verbose;		/* verbose mode if set */
>> +static int quiet;		/* quiet mode if set */
>> +static int num_rpt;		/* ping number, how often to print message */
>> +static int chld_wait;	/* max time to wait between writes, 1 == no wait */
>> +static int parent_wait;	/* max time to wait between reads, 1 == no wait */
>> +static int ndelay = O_NDELAY;	/* additional flag to open */
>> +static char *writebuf;
>> +static char *readbuf;
>> +static char pname[PATH_MAX];	/* contains the name of the unamed pipe */
>> +static char *blk_type = NON_BLOCKING_IO; /* blocking i/o or not */
>> +static char *pipe_type;		/* type of pipe under test */
>> +static int fds[2];		/* un-named pipe fds */
>> +static int read_fd;
>> +static int write_fd;
>> +static int empty_read;
>> +static int format = HEX;
>> +static int format_size = -1;
>> +static int iotype;		/* sync io */
>> +static int sem_id;
>> +static struct sembuf sem_op;
> Some of these variables could be local: "fds" only in setup(),
> "sem_op" could be local in do_child() and do_parent().
>
> I'd suggest to also split this list of global variables, so it's clear
> which are modified as part of setup and which change value
> also throughout the run.
OK, it's reasonable, thanks.

Regards,
Xiaoguang Wang
>
> Split of code into functions looked OK to me.
>
> Regards,
> Jan
> .
>


------------------------------------------------------------------------------
Put Bad Developers to Shame
Dominate Development with Jenkins Continuous Integration
Continuously Automate Build, Test & Deployment 
Start a new project now. Try Jenkins in the cloud.
http://p.sf.net/sfu/13600_Cloudbees
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list