473,326 Members | 2,076 Online
Bytes | Software Development & Data Engineering Community
Post Job

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 473,326 software developers and data experts.

how to remove code duplication

I have created a program which creates and renames files. I have
described everything in comments. All I have is the
cod-duplication. function like fopen, sprint and fwrite are being called
again and again.

I know to remove code-duplication I have to make functions and pass
arguments to them but I am not able to think of a way doing it. Can you
post some example for me, out of this code:


/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.0
*
Each log file is of fixed size called LOG_SIZE, if input from stdin is more than
LOG_SIZE then we will write the date to 2nd file. file naming convention is %d.log,
e.g. 1.log, 2.log , 3.log etc.

We have 3 scenerios:

1) if we already have 0.log and (new incoming data + sizeof(0.log)) is less than
LOG_SIZE then we will write the data to 0.log. otherwise we wil go to scenerio
number 2.
2) if we have 0.log filled with size LOG_SIZE, then we will rename it to 1.log and
create a new file 0.log and feed the new data into it. Hence largest the number
oldest is the file.

3) Number of log-files are to be kept less than LOG_CNT_MAX, if we hit the LOG_CNT_MAX
then we we will delete the oldest file, rename the files to new numbers and create
a new file to write data. e.g. if LOG_CNT_MAX = 3 and we have 0.log, 1.log, 2.log
filled with data of LOG_SIZE, then we will not create 3.log but we will delete 2.log
and rename 1.log to 2.log and 0.log to 1.log respectively and create a new 0.log to
write data.

thats the whole basic idea of my logging system.
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_NAME "%d.log"
#define LOG_DIR "."
#define TRUE 1
#define FALSE 0

enum { LOG_NAME_SIZE = 6, LOG_SIZE = 6, LOG_CNT_MAX = 30, LOG_BASE_NUM = 0 };
int find_log_num( const char * );

/* LOG_NAME_SIZE is the size of LOG_NAME in bytes

* LOG_CNT_MAX is the maximum number of log files to be kept on the system

* LOG_SIZE is the size of log file.

* LOG_INPUT_MAX is also the maximum input that a client-log is supposed to send.
* if the log-input is more than LOG_INPUT_MAX, then it means we are not having a log file
* but either a system bug or malicious intentions of some SPAMMER. So refuse the baby to
* access your system ;)
*

* If you insist on using this file as a separate program running in main() then make sure
LOG_CNT_MAX and "i" in main for loop are in COMPLETE HARMONY
*
*
*/
int main( void )
{
// char log_arr[LOG_SIZE];
const char log_recv_arr[] = "Love\n";
char log_name[LOG_NAME_SIZE];
long log_size;
int log_cnt;
FILE *fp;
size_t log_recv_size;
int BACKUP;

/* directory and file manipulation variables */
struct stat statbuf;

/* initialize arrays */
memset( log_name, '\0', LOG_NAME_SIZE);
log_size = 0;
log_recv_size = strlen( log_recv_arr );
// printf("INPUT IS: %s\n\n", log_recv_arr);
log_cnt = 0;
printf("log_cnt = %d\n", log_cnt);

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
if( sprintf(log_name, LOG_NAME, i) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}

if( ! (stat(log_name, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}
}

printf("--- log_cnt = %d\n", log_cnt);
if( log_cnt )
{
BACKUP = TRUE;
}
else
{
BACKUP = FALSE;
}
// we have the log_cnt, which means file exists, so we can find its size
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SNPRINTF ERROR - after search loop");
exit( EXIT_FAILURE );
}

if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

printf("%s = %ld\n", log_name, log_size);

/* start Logging */
for(int i = 0 ; i != 8; ++i)
{
printf("\n\nLOOP #%d\n", i);

if( !log_cnt ) // S1 -- begin
{
/* There are 2 sub-cases here:

1) 0.log does not exist, we create a new one.

since we created a fresh file, hence we are sure that size of fresh file
is == LOG_SIZE, so we don't need any check before writing to it. after
creating the file we need to check how much data we have written.

filesize == LOG_SIZE --create a new filename, probably 1.log
filesize < LOG_SIZE --go for input once more
2) 0.log exists but size is less than LOG_SIZE

(filesize + received data) <= LOGSIZE --write data
ELSE --create anew file
*/
printf("--------------- Into SCENERIO (1) :: log_cnt == 0 -----------------\n\n");

// log_size == 0 means we have no backup files, hence we will do a fresh log
// or else we can use BACKUP flag
if( ! log_size ) // S1 1st case
{
printf("Entered 1st case\n");
printf("File does not exist, Creating a new one\n");

// In future, it will be a function: create_log_name( log_name );
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}

// In future, it will be a function: write_data( log_name );
if( NULL == (fp = fopen(log_name, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 219");
exit( EXIT_FAILURE );
}
// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long)statbuf.st_size;
}

printf(" %s written = %ld\n\n", log_name, log_size);

if( log_size LOG_SIZE )
{
perror("YOU STUPID MORON .... 1st case (END)");
exit( EXIT_FAILURE );
}
} // S1 1st case

else if( (log_size + log_recv_size) <= LOG_SIZE )//S1 --2nd case
{
printf("Entered 2nd case\n");
printf("File exists\n");
printf("%s size = %ld\n", log_name, log_size);
/* open file and append the data */
if( ! (fp = fopen(log_name, "a")) )
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}

/* update log_size */
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("%s written = %ld\n\n", log_name, log_size);
} // S1 <-- 2nd case

else if( (log_size + log_recv_size) LOG_SIZE ) // S1 --3rd case
{
printf("Entered 3rd case\n");
printf("%s = %ld\n", log_name, log_size);
++log_cnt;
}

} // if( !log_cnt ) S1 <--- END
// SCENERIO (2)
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )//log_cnt < LOG_CNT_MAX ) // S2 --begin
{

/* Entering into this condition means we are definitely sure that 0.log has been
either filled or size of (new input + size of (0.log)) is LOG_SIZE.

NOTE: 1st of all we will move the log files. we will reach at 1.log with final move.
we will create an empty 0.log file and then proceed as usual.

sinec ewe have fresh file, 0.log, we can fit all the data into it as, because of the ENUM
constants above, we are sure the data will always be <= LOG_SIZE

*/

printf("------------------- Into SCENERIO (2) -------------------\n");
printf("log_cnt = %d\n", log_cnt);

if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("Oldest %s exists with size = %ld bytes\n", log_name, log_size);
printf( "BACKUP = %d\n", BACKUP);

/* move_log( log_cnt - 1 )
log_create( 0 );
*/

//================================================== ==================================
//========================== RENAME MECHANISM ========================================

int backup_cnt;

if( BACKUP )
{
backup_cnt = log_cnt + 1;
}
else
{
backup_cnt = log_cnt;
}

char temp_oldname[LOG_NAME_SIZE];
char temp_newname[LOG_NAME_SIZE];

for( int i = backup_cnt; (i LOG_BASE_NUM) && (i < LOG_CNT_MAX); --i )
{
if( sprintf(temp_oldname, LOG_NAME, i-1) < 0 )
{
perror("SPRINTF ERROR oldname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

if( sprintf(temp_newname, LOG_NAME, i ) < 0 )
{
perror("SPRINTF ERROR newname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

printf("i = %d\n", i );
printf("temp_oldname = %s\n", temp_oldname);
printf("temp_newname = %s\n", temp_newname);

if( rename(temp_oldname, temp_newname) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
printf("renamed <%sto <%s>\n", temp_oldname, temp_newname);
//sleep(2);
}
}
// create 0.log
if( sprintf(log_name, LOG_NAME, LOG_BASE_NUM) < 0 )
{
perror("SPRINTF ERROR :: S2 --case b\n");
exit( EXIT_FAILURE );
}
// open file and append the data
if( ! (fp = fopen(log_name, "a")) ) // S2 -- case a
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}

// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("%s written = %ld\n\n", log_name, log_size);

// update the log_cnt
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++log_cnt;
}

//========================== RENAME MECHANISM ========================================
//================================================== ==================================
} // S2 begin ends
// SCENERIO (3)
else // log_cnt >= LOG_CNT_MAX
{
printf("------------------- Into SCENERIO (3) -------------------\n");

printf("log_cnt = %d, LOG_CNT_MAX = %d\n", log_cnt, LOG_CNT_MAX);

if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("Oldest %s exists with size = %ld bytes\n", log_name, log_size);
printf( "BACKUP = %d\n", BACKUP);

/* well here we are log_cnt >= LOG_CNT_MX. We will simply do 3 things:

1) Delete the oldest file, which is (LOG_CNT_MAX - 1).log
2) Rename all files
3) create 0.log name
4) create 0.log file
5) write data
*/
//================================================== ==================================
//========================== RENAME MECHANISM ========================================

int backup_cnt = log_cnt - 1;

char temp_oldname[LOG_NAME_SIZE];
char temp_newname[LOG_NAME_SIZE];

for( int i = backup_cnt; (i LOG_BASE_NUM) && (i < LOG_CNT_MAX); --i )
{
if( sprintf(temp_oldname, LOG_NAME, i-1) < 0 )
{
perror("SPRINTF ERROR oldname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

if( sprintf(temp_newname, LOG_NAME, i ) < 0 )
{
perror("SPRINTF ERROR newname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

printf("i = %d\n", i );
printf("temp_oldname = %s\n", temp_oldname);
printf("temp_newname = %s\n", temp_newname);

if( rename(temp_oldname, temp_newname) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
printf("renamed <%sto <%s>\n", temp_oldname, temp_newname);
//sleep(2);
}
}
// create 0.log
if( sprintf(log_name, LOG_NAME, LOG_BASE_NUM) < 0 )
{
perror("SPRINTF ERROR :: S2 --case b\n");
exit( EXIT_FAILURE );
}
// open file and append the data
if( ! (fp = fopen(log_name, "a")) ) // S2 -- case a
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}

// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("%s written = %ld\n\n", log_name, log_size);

// No need to update the log_cnt as log_cnt == LOG_CNT_MAX
//========================== RENAME MECHANISM ========================================
//================================================== ==================================
/*
if( ! (stat(log_name_old, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
printf("File %s exists with size = %ld bytes\n", log_name, log_size);
printf("// code yet to be written\n\n");
*/
}

} // for( ... ) loop
return 0;
}
















// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^
// R . E. N. A. M. E F. I. L. E. S
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~
/*
char temp_oldname[LOG_NAME_SIZE];
char temp_newname[LOG_NAME_SIZE];

for( int i = log_cnt; i <= 0; --i )
{
if( sprintf(temp_oldname, LOG_NAME, i) < 0 )
{
perror("SPRINTF ERROR oldname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

if( sprintf(temp_newname, LOG_NAME, (i + 1) ) < 0 )
{
perror("SPRINTF ERROR newname :: S2 --case b\n");
exit( EXIT_FAILURE );
}

printf("i = %d\n", i );
printf("temp_oldname = %s\n", temp_oldname);
printf("temp_newname = %s\n", temp_newname);

printf("---------------------------------------------------------------------------------\n\n");
system("ls");
if( rename(temp_oldname, temp_newname) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
system("ls");
printf("---------------------------------------------------------------------------------\n\n");
}
// create 0.log
if( sprintf(log_name, LOG_NAME, 0) < 0 )
{
perror("SPRINTF ERROR :: S2 --case b\n");
exit( EXIT_FAILURE );
}
// open file and append the data
if( ! (fp = fopen(log_name, "a")) ) // S2 -- case a
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}

// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("%s written = %ld\n\n", log_name, log_size);

// update the log_cnt
if( log_size >= LOG_SIZE )
{
++log_cnt;
printf("log_cnt = %d\n", log_cnt);
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR :: S2 --case b (END) \n");
exit( EXIT_FAILURE );
}
}
} // S2 <-- case b

}
*/
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^
// R . E. N. A. M. E F. I. L. E. S
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~


--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 11 '08 #1
61 3186
On 11 Aug, 07:46, arnuld <sunr...@invalid.addresswrote:
I have created a program which creates and renames files. I have
described everything in comments. *All I have is the
cod-duplication. function like fopen, sprint and fwrite are being called
again and again.

I know to remove code-duplication I have to make functions and pass
arguments to them but I am not able to think of a way doing it. Can you
post some example for me, out of this code:
<snip waffly comments>

<snip code>

you have a 600-line main function!!!!

As a rule of thumb start to ask yourself if the function could be
split
at say 10-20 lines. A 50 line function isn't always wrong (eg. big
switch
statement) but an alarm bell should be ringing.

Step 1 write a test program that verifies your program is correct.
Perhaps base it on this skeleton

/* runs a test on the logger
the log directory contains the in files before the test
the data from the data directory is read
the log directory should containing the same files as the out
directory
*/
void run_log_test (const char *test_name)
{
char test_dir_in [80];
char test_dir_out [80];
sprintf (test_dir_in, "test%s/in", test_name);
sprintf (test_dir_dat, "test%s/data", test_name);
sprintf (test_dir_out, "test%s/out", test_name);

clear_log_directory();
copy (test_dir_in, log);
read_data (test_dir_dat);
match_dir (log, test_dir_out); /* calls assert if any files
don't match */
}

void test_logger()
{
run_log_test ("1");
run_log_test ("2");
run_log_test ("3");
}

it should be designed so you don't have to check data
by hand- the program does it all for you.

Write some tests that verify it behaves correctly for various
scenarios.

eg. no 0.log short input (no file roll-over)
0.log present short input
medium input (rolls over to 1.log)
long input (less than log max files created)
very long input (files have to be deleted)
anything else that might stress your program

step 2 run the tests
step 3 identify short bits of repetitive code.
create functions from the repeated code.
call the functions where the code was repeated.

step 4 run the test

At the moment your code is so tangled its hard to pick out
simple bits of repetitive code.

I'd "wrapper" calls like these

if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}

void make_name (char *log_name, int log_cnt)
{
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}
}

which doesn't save you much but you call them a lot.
Similarly fopen(), fwrite(), fclose().

As a rule of thumb if you find yourself using block comments
like

//
================================================== =======================*
===========
//========================== RENAME MECHANISM ========================================

then that is *begging* to be turned into a function. Even
if it only appears once it will simplify you main.

This seems to appear a few times

// open file and append the data
if( ! (fp = fopen(log_name, "a")) ) // S2 -- case a
{
perror("FOPEN() ERROR whne log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fwrite(log_recv_arr, 1, log_recv_size, fp) != log_recv_size )
{
perror("FWRIRE ERROR in log_cnt == 0");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 187");
exit( EXIT_FAILURE );
}

// update log_size
if( ! (stat(log_name, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}

printf("%s written = %ld\n\n", log_name, log_size);

// update the log_cnt
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++log_cnt;
}

Try to identify higher level funtionality. The following code is
uncompiled.
int main (void)
{
FILE *stream;
get_log_stream(stream); /* TBD */
process_log_data(stream);
return 0;
}

void process_log_data(FILE *stream)
{
int log_cnt = 0;
int i;
FILE *curr_log;

while (more_stuff)
{
if (!log_file_exists(0))
create_log(0);

curr_log = open_log(0);

if (store_records (curr_log, stream) == STREAM_EMPTY)
return;

/* current log full- move to next log */
fclose(curr_log);

/* rename logs */
for (i = log_cnt; i >= 0 i--)
rename_log (i, i + 1);

log_cnt++;

/* TBD handle too many log files */
}
}
/* store recored in the current log until either
- stream exhauseted
- log full
*/

Store_result store_records (curr_log, stream)
{
/* TBD */
}

/* primitives */
int log_file_exists (int n)
{
FILE *f;
make_name(n)
if ((f = fopen(make_name(n), "w") == NULL)
return FALSE;
fclose(f)
return TRUE;
}

char *make_name(int n)
{
static name[80];
sprintf (name, LOG_NAME, n);
return name;
}
still rather messy but I probably did it the opposite
way to you. You wrote pages of code and then thought
"how do I modularise this?". I thought "what is the program
trying to do it and what functions whould make it
easy for me to write it".

I quickly realise I want to open files, rename files etc.

So at the moment process_log_data represents the guts of the program.
The log renmae loop should probably go into a function
and the handling of too many logs.
--
Nick Keighley
Aug 11 '08 #2
On 2008-08-11, Nick Keighley <ni******************@hotmail.comwrote:
I'd "wrapper" calls like these

if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}

void make_name (char *log_name, int log_cnt)
{
if( sprintf(log_name, LOG_NAME, log_cnt) < 0 )
{
perror("SPRINTF ERROR -- near line 217");
exit( EXIT_FAILURE );
}
}

which doesn't save you much but you call them a lot.
Similarly fopen(), fwrite(), fclose().
What *would* save him a lot, if the code is designed to always display
an error message and exit, would be to have a macro/function combination:

/* Start */

#ifdef DEBUG
#define DBG(msg) _err(msg, "DEBUG", __func__, 0)
#else
#define DBG(msg)
#endif
#define ERR(msg) _err(msg, "ERROR", __func__, 0)
#define DIE(msg) _err(msg, "FATAL", __func__, 1)
void _err(const char *error_msg,
const char *severity,
const char *caller,
char bool_terminate)
{
if(error_msg != NULL)
fprintf(stderr, "%s: %s: %s\n", severity, caller, error_msg);
if(bool_terminate)
exit(EXIT_FAILURE);
}

/* End */

For most projects, I can put that verbatim into a header file, and from
there almost all of my error-checking can be reduced to something like:

if((buff = malloc(sizeof buff)) == NULL)
DIE("Out of memory!");

Then if I decided not to terminate, but to change tactics and attempt
using a pre-allocated buffer, or a file, or whatever, it's a simple
matter to change DIE to an ERR or DBG, which won't terminate the
program, and then insert my recovery code.
On a related note, it's generally bad code design to terminate a
program on error, unless the error in question is, for example, a
failure to open a critical configuration file. Depending how important
stability is (or how volatile your environment is!), you can use stdin
or stdout in lieu of a failed file access, use static buffers (or disk
files) in lieu of failed memory allocation, and in many cases, you can
skip over sections of code entirely without critically hurting program
flow.

If you prompt the user in these cases, he'll likely be very impressed,
and relieved that any work the program was in the middle of was not
lost. Having said that, don't go too far: having your program output
"Printing contents of program memory - please take page to another
computer, run the program with the --recover flag, and type it all in"
is NOT recommended!

Aug 11 '08 #3
arnuld wrote:
>
I have created a program which creates and renames files. I have
described everything in comments. All I have is the
cod-duplication. function like fopen, sprint and fwrite are being
called again and again.

I know to remove code-duplication I have to make functions and
pass arguments to them but I am not able to think of a way doing
it. Can you post some example for me, out of this code:
Throw it away. Write simple code, with short functions. Such as:

int main(int argc, char **argv) {
if (initialize(...)) {
if (readdata(...)) {
if (writedata(...)) {
if (goodclose(...)) return 0;
}
}
}
return EXIT_FAILURE;
}

Now you have to write the functions referenced, and set up the
parameters passed. You also have to add the necessary #includes,
and appropriate data objects. You can make temporary function to
test things, such as:

int initialize(...) {
puts("Initializing");
return 1;
}

and test things. The idea is to start compiling and testing as
soon as possible, then revise to make it do what you want. Also
note how easy it is to separate things out into separate source
files that do useful things.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.
Aug 11 '08 #4
On 11 Aug, 23:59, CBFalconer <cbfalco...@yahoo.comwrote:
arnuld wrote:
I have created a program which creates and renames files. I have
described everything in comments. *All I have is the
cod-duplication. function like fopen, sprint and fwrite are being
called again and again.
I know to remove code-duplication I have to make functions and
pass arguments to them but I am not able to think of a way doing
it. Can you post some example for me, out of this code:

Throw it away. *Write simple code, with short functions.
yes, that's perhaps what I should have said.

<snip>

--
Nick Keighley
Aug 12 '08 #5
On Mon, 11 Aug 2008 18:59:25 -0400, CBFalconer wrote:
Throw it away. Write simple code, with short functions. Such as:

int main(int argc, char **argv) {
if (initialize(...)) {
if (readdata(...)) {
if (writedata(...)) {
if (goodclose(...)) return 0;
}
}
}
return EXIT_FAILURE;
}
... SNIP....
note how easy it is to separate things out into separate source
files that do useful things.
I know it and I wanted to do it like that and it looks much better than
mine . K&R2 follows the same pattern but I don't understand why my mind
does not think like this. I tried hard but have to came up with my own way.
--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page
Aug 18 '08 #6
On Mon, 11 Aug 2008 02:00:08 -0700, Nick Keighley wrote:
you have a 600-line main function!!!!
As a rule of thumb start to ask yourself if the function could be
split at say 10-20 lines. A 50 line function isn't always wrong (eg.
big switch statement) but an alarm bell should be ringing.
okay, does that rule applies as same to main() ?

Step 1 write a test program that verifies your program is correct.
Perhaps base it on this skeleton
.. SNIP....
it should be designed so you don't have to check data
by hand- the program does it all for you.

I tried it but my program is interactive. I mean it needs to run the
executable from command line and then check the output, which are
text files on hard disk. I am not able to think of doing that using a
program.
.... SNIP....
You
wrote pages of code and then thought "how do I modularise this?". I
thought "what is the program trying to do it and what functions whould
make it easy for me to write it".
I want to do the same, its the way I have seen in K&R2 and I liked it. It
is easier to design, maintain and debug. I don't know, I can;t make it
anyhow, I thought hard about it but nothing came over my mind. May be I
need more experience to make such design. 2nd, I have tried hard to put
the functionality into the functions and making function calls from main.
Though there are still much more lines in main but many of them are
printf() calls, a help for debugging. I will remove them as soon as I get
my code to design and work properly. Here is my new version, labeled 1.1,
tell me where I need to make more of changes/abstraction:


/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.1
*
Each log file is of fixed size called LOG_SIZE, if input from stdin is more than
LOG_SIZE then we will write the date to 2nd file. file naming convention is %d.log,
e.g. 1.log, 2.log , 3.log etc.

We have 3 scenerios:

1) if we already have 0.log and (new incoming data + sizeof(0.log)) is less than
LOG_SIZE then we will write the data to 0.log. otherwise we wil go to scenerio
number 2.
2) if we have 0.log filled with size LOG_SIZE, then we will rename it to 1.log and
create a new file 0.log and feed the new data into it. Hence largest the number
oldest is the file.

3) Number of log-files are to be kept less than LOG_CNT_MAX, if we hit the LOG_CNT_MAX
then we we will delete the oldest file, rename the files to new numbers and create
a new file to write data. e.g. if LOG_CNT_MAX = 3 and we have 0.log, 1.log, 2.log
filled with data of LOG_SIZE, then we will not create 3.log but we will delete 2.log
and rename 1.log to 2.log and 0.log to 1.log respectively and create a new 0.log to
write data.

thats the whole basic idea of my logging system.
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/"
#define LOG_NAME "%d.log"

enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10, LOG_CNT_MAX = 6, LOG_BASE_NUM = 0 };
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );
int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );

char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "TCP/IP\n";

/* LOG_NAME_SIZE is the size of LOG_NAME in bytes

* LOG_CNT_MAX is the maximum number of log files to be kept on the system

* LOG_SIZE is the size of log file.

* LOG_SIZE is also the maximum input that a client-log is supposed to send.
* if the log-input is more than LOG_SIZE, then it means we are not having a log file
* but either a system bug or malicious intentions of some SPAMMER. So refuse the baby to
* access your system ;)
*

* If you insist on using this file as a separate program running in main() then make sure
* LOG_CNT_MAX and "i" in main for loop are in COMPLETE HARMONY
*
*
*/
int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_SIZE];
long log_size;
int log_cnt;

/* initialize variables*/
memset( log_name, '\0', LOG_NAME_SIZE);
log_size = 0;
const size_t log_recv_size = strlen( log_recv_arr );

log_cnt = 0;
printf("OLD log_cnt = %d\n", log_cnt);

log_cnt = oldest_log( log_path );
printf("NEW log_cnt = %d\n", log_cnt);

// we have the log_cnt, which means file exists, so we can find its size
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );
printf("%s = %ld\n", log_name, log_size);

// start Logging
for(int i = 0 ; i != 3; ++i)
{
printf("\n\nLOOP #%d\n", i);
if( !log_cnt ) // S1 -- begin
{
/* There are 2 sub-cases here:

1) 0.log does not exist, we create a new one.

since we created a fresh file, hence we are sure that size of fresh file
is == LOG_SIZE, so we don't need any check before writing to it. after
creating the file we need to check how much data we have written.

filesize == LOG_SIZE --create a new filename, probably 1.log
filesize < LOG_SIZE --go for input once more
2) 0.log exists but size is less than LOG_SIZE

(filesize + received data) <= LOGSIZE --write data
ELSE --create anew file
*/

printf("--------------- Into SCENERIO (1) :: log_cnt == 0 -----------------\n\n");

// log_size == 0 means we have no backup files, hence we will do a fresh log
// or else we can use BACKUP flag
if( ! log_size ) // S1 1st case
{
printf("Entered 1st case\n");
printf("File does not exist, Creating a new one\n");

create_logname( log_name, log_cnt );
create_logpath( log_path, log_name );
create_log( log_path );

// update log_size
log_size = find_logsize( log_path );
printf("log_size = %ld\n", log_size);
printf(" %s written = %ld\n\n", log_path, log_size);

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n");

if( log_size LOG_SIZE )
{
perror("YOU STUPID MORON .... 1st case (END)");
exit( EXIT_FAILURE );
}
} // S1 1st case

else if( (log_size + log_recv_size) <= LOG_SIZE )//S1 --2nd case
{
printf("Entered 2nd case\n");
printf("File exists\n");
printf("%s size = %ld\n", log_name, log_size);
// create full pathname for the file
strcat( log_path, log_name );
printf("\nFULL PATH and FILENAME = %s\n", log_path);

// open file and append the data
create_log( log_path );

// update log_size
log_size = find_logsize( log_name );
printf("%s written = %ld\n\n", log_path, log_size);

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n");

} // S1 <-- 2nd case

else if( (log_size + log_recv_size) LOG_SIZE ) // S1 --3rd case
{
printf("Entered 3rd case\n");
strcat( log_path, log_name );
printf("%s = %ld\n", log_path, log_size);
++log_cnt;

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n");

}
} // if( !log_cnt ) S1 <--- END
// SCENERIO (2)
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )//log_cnt < LOG_CNT_MAX ) // S2 --begin
{
// Entering into this condition means we are definitely sure that 0.log has been
// either filled or size of (new input + size of (0.log)) is LOG_SIZE.

// NOTE: 1st of all we will move the log files. we will reach at 1.log with final move.
// we will create an empty 0.log file and then proceed as usual.

// sinece we have fresh file, 0.log, we can fit all the data into it as, because of the ENUM
// constants above, we are sure the data will always be <= LOG_SIZE
printf("------------------- Into SCENERIO (2) -------------------\n");
printf("log_cnt = %d\n", log_cnt);

strcat( log_path, log_name );
log_size = find_logsize( log_path );

printf("Oldest %s exists with size = %ld bytes\n", log_name, log_size);
printf( "log_cnt = %d\n", log_cnt);

rename_files( log_cnt );

// clear the log_path
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME );
// all files are renamed now we can create 0.log, as always the latest file
create_logname( log_name, LOG_BASE_NUM );
create_logpath( log_path, log_name );
printf("logpath = %s\n", log_path);

create_log( log_path );

// update log_size
log_size = find_logsize( log_path);
printf("%s written = %ld\n\n", log_path, log_size);

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n");

// update the log_cnt
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++log_cnt;
}

if( log_size LOG_SIZE )
{
perror("STUPID MORON in scenerio 2\n");
exit( EXIT_FAILURE );
}

} // S2 begin ends
// SCENERIO (3)
else // log_cnt >= LOG_CNT_MAX
{
printf("------------------- Into SCENERIO (3) -------------------\n");
printf("log_cnt = %d, LOG_CNT_MAX = %d\n", log_cnt, LOG_CNT_MAX);

log_size = find_logsize( log_name );

printf("Oldest %s exists with size = %ld bytes\n", log_name, log_size);

// well here we are log_cnt >= LOG_CNT_MX. We will simply do 3 things:

// 1) Delete the oldest file, which is (LOG_CNT_MAX - 1).log
// 2) Rename all files
// 3) create 0.log name
// 4) create 0.log file
// 5) write data
rename_files( --log_cnt );

// clear the log path and log name
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME );
// all files are renamed now we can create 0.log, as always the latest file
create_logname( log_name, LOG_BASE_NUM );
create_logpath( log_path, log_name );
printf("logpath = %s\n", log_path);

create_log( log_path );

// update log_size
log_size = find_logsize( log_path);
printf("%s written = %ld\n\n", log_path, log_size);

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n");

// we will no longer update the log_cnt as it has already reached the maximum limit
// BUT we have decreased the log_cnt from log_cnt (which is >= LOG_CNT_MAX) to
// --log_cnt. Hence we need to increase it again, so that only SCENERIO 3 becomes
// true
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++log_cnt;
}

if( log_size LOG_SIZE )
{
perror("What the HELL! happened here in SCENERIO 3 \n");
exit( EXIT_FAILURE );
}
}

} // for( ... ) loop

return 0;
}

// -------------------------------------------------------------------------------
// -------------------------- Functions --------------------------------------------


// ---------------- ** filename and filesize related function ** -------------------
// ------------ create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// ------------------- created the actual, physical log file
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// ------------------- finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// ------------------- find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
printf("i = %d\n", i);
create_logname( log_name, i );
create_logpath( log_path, log_name );
printf("FULL PATH and FILENAME = %s\n", log_path);

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n\n");
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
printf("PATHNAME CLEARED\n\n");

}
return log_cnt;
}


// ------------------- ** File I/O functions ** ----------------------

// ----- create a new file, write data and close the file in the end -----
void create_log( const char *log_path )
{
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );
if( ! (fp = fopen(log_path, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

log_size = find_logsize( log_path );

if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}

if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR near line 219");
exit( EXIT_FAILURE );
}
}

// ------------- rename files ---------------------------
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_SIZE];
char temp_newname[LOG_NAME_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

int backup_cnt;

backup_cnt = log_cnt;
for( int i = backup_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

printf("i = %d, backup_cnt = %d\n", i, backup_cnt );
printf("temp_oldname = %s\n", temp_oldname);
printf("temp_newname = %s\n", temp_newname);
printf("temp_oldpath = %s, size = %d\n", temp_oldpath, (int) strlen(temp_newpath));
printf("temp_newpath = %s, size = %d\n", temp_newpath, (int) strlen(temp_newpath));
if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
printf("renamed <%sto <%s>\n", temp_oldname, temp_newname);
// clear pathnames
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
//sleep(2);
}
}
}


--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 18 '08 #7
arnuld wrote:
>On Mon, 11 Aug 2008 18:59:25 -0400, CBFalconer wrote:
>Throw it away. Write simple code, with short functions. Such as:

int main(int argc, char **argv) {
if (initialize(...)) {
if (readdata(...)) {
if (writedata(...)) {
if (goodclose(...)) return 0;
}
}
}
return EXIT_FAILURE;
}
>... SNIP....
>note how easy it is to separate things out into separate source
files that do useful things.

I know it and I wanted to do it like that and it looks much better
than mine . K&R2 follows the same pattern but I don't understand why
my mind does not think like this. I tried hard but have to came up
with my own way.
You have to attack a problem in terms of abstract independent or
semi-independent entities that cooperate to solve it. Only *after* the
design has been fully worked should you switch to a language-specific
mode of thought (yea, I know, that sentence can be nitpicked, but I
hope arnuld gets the point.)

Aug 18 '08 #8
On 18 Aug, 08:03, arnuld <sunr...@invalid.addresswrote:
*On Mon, 11 Aug 2008 02:00:08 -0700, Nick Keighley wrote:
you have a 600-line main function!!!!
As a rule of thumb start to ask yourself if the function could be
split at say 10-20 lines. A 50 line function isn't always wrong (eg.
big switch statement) but an alarm bell should be ringing.

okay, does that rule applies as same to main() ?
yes, hence "you have a 600-line main function!!!!".
I try to have almost nothing in my main(). If the argument
handling is simple it goes in main(). main() is often little
more than 1 or 2 function calls.

Step 1 write a test program that verifies your program is correct.
Perhaps base it on this skeleton
.. SNIP....
it should be designed so you don't have to check data
by hand- the program does it all for you.

I tried it but my program is interactive.
you need to separate the interactive bits from the guts
of the program. Why is it interactive?

I mean it needs to run the
executable from command line and then check the output, which are
text files on hard disk. I am not able to think of doing that using a
program.
write a function to compare two files. Return true if they match.

int is_file_equal (FILE *f1, FILE* f2)
{
/* compare files. Use fgets() and strcmp() */
}

I gave you an outline of a test harness

You
wrote pages of code and then thought "how do I modularise this?". I
thought "what is the program trying to do it and what functions whould
make it easy for me to write it".

I want to do the same, its the way I have seen in K&R2 and I liked it. It
is easier to design, maintain and debug. I don't know, I can;t make it
anyhow, I thought hard about it but nothing came over my mind.
I gave you an outline. You might want to try top down functional
decomposition. Start with the program's goal and write a page of
code or pseudo code to do that. Don't worry about the guts
of the functions you call at first. Then take one of the incomplete
functions and write a page of code to do *that*. When all the
functions
are written you have your program. This *is* hard to teach (I spent
some time thinking there was some magic methodology that I hadn't yet
found). Read good code and practice practice practice.

Top down *does* have its problems. You don't have a working program
until you finish and it is easy to wonder directionless or to end up
with a bunch of hard (or impossible) to implement functions at the
bottom.
But you are so into low level detail you need to pull back and think
more abstractly.

May be I
need more experience to make such design. 2nd, I have tried hard to put
the functionality into the functions and making function calls from main.
yes, but you didn't write any functions of your own.
Though there are still much more lines in main but many of them are
printf() calls, a help for debugging. I will remove them as soon as I get
my code to design and work properly. Here is my new version, labeled 1.1,
tell me where I need to make more of changes/abstraction:
this looks so similar to what you posted before that I can't be
bothered to read it.

- build a test harness
- you have a 600 line main() program. Split it.
- identify some common subroutines. open_file(),
rename_file(), delete_file()
- identify and implement some abstractions. write_to_log(),
handle_full_log(), handle_too_many_logs()

--
Nick Keighley

Aug 18 '08 #9
On 18 Aug, 10:40, Nick Keighley <nick_keighley_nos...@hotmail.com>
wrote:
On 18 Aug, 08:03, arnuld <sunr...@invalid.addresswrote:
*On Mon, 11 Aug 2008 02:00:08 -0700, Nick Keighley wrote:
you have a 600-line main function!!!!
As a rule of thumb start to ask yourself if the function could be
split at say 10-20 lines. A 50 line function isn't always wrong (eg.
big switch statement) but an alarm bell should be ringing.
okay, does that rule applies as same to main() ?

yes, hence "you have a 600-line main function!!!!".
I try to have almost nothing in my main(). If the argument
handling is simple it goes in main(). main() is often little
more than 1 or 2 function calls.
Step 1 write a test program that verifies your program is correct.
Perhaps base it on this skeleton
.. SNIP....
it should be designed so you don't have to check data
by hand- the program does it all for you.
I tried it but my program is interactive.

you need to separate the interactive bits from the guts
of the program. Why is it interactive?
I mean it needs to run the
executable from command line and then check the output, which are
text files on hard disk. I am not able to think of doing that using a
program.

write a function to compare two files. Return true if they match.

int is_file_equal (FILE *f1, FILE* f2)
{
* * /* compare files. Use fgets() and strcmp() */

}

I gave you an outline of a test harness
You
wrote pages of code and then thought "how do I modularise this?". I
thought "what is the program trying to do it and what functions whould
make it easy for me to write it".
I want to do the same, its the way I have seen in K&R2 and I liked it. It
is easier to design, maintain and debug. I don't know, I can;t make it
anyhow, I thought hard about it but nothing came over my mind.

I gave you an outline. You might want to try top down functional
decomposition. Start with the program's goal and write a page of
code or pseudo code to do that. Don't worry about the guts
of the functions you call at first. Then take one of the incomplete
functions and write a page of code to do *that*. When all the
functions
are written you have your program. This *is* hard to teach (I spent
some time thinking there was some magic methodology that I hadn't yet
found). Read good code and practice practice practice.

Top down *does* have its problems. You don't have a working program
until you finish and it is easy to wonder directionless or to end up
with a bunch of hard (or impossible) to implement functions at the
bottom.
But you are so into low level detail you need to pull back and think
more abstractly.
May be I
need more experience to make such design. 2nd, I have tried hard to put
the functionality into the functions and making function calls from main.

yes, but you didn't write any functions of your own.
Sorry. This is untrue. You did write some functions of your own.
Though there are still much more lines in main but many of them are
printf() calls, a help for debugging. I will remove them as soon as I get
my code to design and work properly. Here is my new version, labeled 1.1,
tell me where I need to make more of changes/abstraction:

this looks so similar to what you posted before that I can't be
bothered to read it.
... and I should have. You do have some abstraction. Your main
function
is still very large though.

>
- build a test harness
- you have a 600 line main() program. Split it.
- identify some common subroutines. open_file(),
* rename_file(), delete_file()
- identify and implement some abstractions. write_to_log(),
* handle_full_log(), handle_too_many_logs()

--
Nick Keighley

Aug 18 '08 #10
On 18 Aug, 08:03, arnuld <sunr...@invalid.addresswrote:

taking a second look at your code. The bits marked SCENARIO 1, 2 etc.
look to share some common code. Try stripping the printf()s out
and you may be able to see the "wood from the trees". That is
see the common code. For instance I think the structure
is more like this

LOOP WHILE data available
IF 0.log does not exist
create 0.log file
END IF

add records to 0.log
END LOOP

this doesn't handle overflow but it does share code
between scenarios.

--
Nick Keighley
Aug 18 '08 #11
On Mon, 18 Aug 2008 02:47:49 -0700, Nick Keighley wrote:
... and I should have. You do have some abstraction. Your main
function is still very large though.

okay, here is the smallest main() I have developed, 56 lines only, version
1.2. Tell me if we need more abstraction and where:


/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.2
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
#define LOG_NAME "%d.log"

enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10, LOG_CNT_MAX = 6, LOG_BASE_NUM = 0 };

int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int create_rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );

char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "SCTP/IP\n";
int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_SIZE];
long log_size;
int log_cnt, *log_cnt_p;

memset( log_name, '\0', LOG_NAME_SIZE);
log_size = 0;
const size_t log_recv_size = strlen( log_recv_arr );

log_cnt = oldest_log( log_path );
log_cnt_p = &log_cnt;
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );
for(int i = 0 ; i != 3; ++i)
{ // SCENERIO 1
if( !log_cnt )
{
if( ! log_size )
{
log_size = create_logfile_zero( log_name, log_path, log_cnt );
}
else if( (log_size + log_recv_size) <= LOG_SIZE )
{
log_size = create_logfile( log_name, log_path );
}
else // else if( (log_size + log_recv_size) LOG_SIZE )
{
++log_cnt;
strcpy(log_path, LOG_DIR);
}
}

// SCENERIO (2)
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
{
create_logpath( log_path, log_name );
create_rename_logs( log_name, log_path, log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );
}

// SCENERIO (3)
else // log_cnt >= LOG_CNT_MAX
{
create_rename_logs( log_name, log_path, --log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );
}

} // for( ... ) loop

return 0;
}

// ---------------- ** Abstracted Functions ** --------------------

// rename and create logs
int create_rename_logs( char *log_name, char *log_path, int log_cnt )
{
int log_size;

log_size = find_logsize( log_path );

rename_files( log_cnt );
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME );
return log_size;
}
// create logs other than 0.log
int create_logfile( char *log_name, char *log_path )
{
int log_size;

create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

// clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}

// create 0.log
int create_logfile_zero( char *log_name, char *log_path, int log_num )
{
int log_size;

create_logname( log_name, log_num );
create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

//clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}


// ---------------- ** filename and filesize related function ** -------------------

// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// created the full log path name
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
create_logname( log_name, i );
create_logpath( log_path, log_name );

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
}
return log_cnt;
}
// update the log count
void update_logcount( int log_size, int log_recv_size, int *log_cnt_p )
{
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++*log_cnt_p;
}
}
// ------------------- ** File I/O functions ** ----------------------

// create a new file, write data and close the file in the end
void create_log( const char *log_path )
{
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );
if( ! (fp = fopen(log_path, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

log_size = find_logsize( log_path );

if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}

if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR");
exit( EXIT_FAILURE );
}
}

// rename files
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_SIZE];
char temp_newname[LOG_NAME_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

for( int i = log_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
}
}
}


--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 19 '08 #12
arnuld wrote:
>On Mon, 18 Aug 2008 02:47:49 -0700, Nick Keighley wrote:
>... and I should have. You do have some abstraction. Your main
function is still very large though.


okay, here is the smallest main() I have developed, 56 lines only,
version 1.2. Tell me if we need more abstraction and where:

/* The Logging program:
* A programs that will take input from stdin and put that into log
* files. using C99 (GNU-Linux/UNIX specific extensions may be there)
* VERSION 1.2
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
Bad idea to hardwire absolute paths like this. Better to use a relative
path.
#define LOG_NAME "%d.log"

enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10, LOG_CNT_MAX
= 6, LOG_BASE_NUM = 0 };
What's the advantage to using enum here. It's simply misleading.
int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int create_rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );
If these interfaces are meant to be public, then giving descriptive
names to the parameters is essential.
char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "SCTP/IP\n";
int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
It's dicey programming practise to shadow filescope objects with block
scope ones.

<snip some code>
// ---------------- ** Abstracted Functions ** --------------------
Better to use C style comments when posting to newsgroups. This comment
of your above line-wrapped and caused a syntax error when I tried to
compile your code. Granted the fix was trivial, but it could have been
worse if many such comments had wrapped.
// rename and create logs
int create_rename_logs( char *log_name, char *log_path, int log_cnt )
The choice of name for this function seem pretty poor to me.

<snip more code>
// ---------------- ** filename and filesize related function **
-------------------
See what I mean?
// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
I would actually extract out the diagnostic routines. This is more work
up-front, but if done properly you get to re-use the same
infrastructure for other programs too. When you display an error
message it's not significantly more trouble to indicate the file name,
function name and line number too. Your error message in unhelpful and
80s style.
exit( EXIT_FAILURE );
Generally you should leave it upto higher level code to decide what to
do on error. Leaf functions should simply indicate status.

<snip even more code>
if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
Maybe you should consider assert instead of cute messages like this?

<snip rest>

Aug 19 '08 #13
santosh wrote:
arnuld wrote:
....
>// ---------------- ** Abstracted Functions ** --------------------

Better to use C style comments when posting to newsgroups.
That is a C-style comment. You might mean C90-style comment?
Aug 19 '08 #14
On 19 Aug, 09:03, arnuld <sunr...@invalid.addresswrote:
On Mon, 18 Aug 2008 02:47:49 -0700, Nick Keighley wrote:
... and I should have. You do have some abstraction. Your main
function is still very large though.

okay, here is the smallest main() I have developed, 56 lines only, version
1.2. Tell me if we need more abstraction and where:
<snip code>

not bad. I might well leave it there. I'm assuming the code
actually works! I haven't compiled it, reviewed it or tested it.
If it's bug free and your Requirments Authority (which might be
you with another hat) isn't demanding any more changes then
I'd stop working on it. If it *does* have bugs or new requirements
it should be much easier to see what is going on and modify it.
--
Nick Keighley
Aug 19 '08 #15
James Kuyper wrote:
santosh wrote:
>arnuld wrote:
...
>>// ---------------- ** Abstracted Functions ** --------------------

Better to use C style comments when posting to newsgroups.

That is a C-style comment. You might mean C90-style comment?
Yes.

Aug 19 '08 #16
On 19 Aug, 09:03, arnuld <sunr...@invalid.addresswrote:
>
okay, here is the smallest main() I have developed, 56 lines only, version
1.2. Tell me if we need more abstraction and where:
<snip>

A general suggestion--instead of writing block comments followed
by code, put the block in a function. For example, you write:
* for(int i = 0 ; i != 3; ++i)
* * { // SCENERIO 1
if( !log_cnt )
<snip bunch of code>
* * * // SCENERIO (2)
* * * else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
<snip more code>
* * * // SCENERIO (3)
else // log_cnt >= LOG_CNT_MAX
<snip more code>
* * } // for( ... ) loop
It would be much clearer of you wrote this as:
for( int i = 0; i < 3; i++ ) {
if( !log_cnt )
create_first_logfile( ... );
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
append_current_log( ... );
else
rotate_logs( ... ) ;
}

but all of that should be the single line:
append_data_to_log( ... );

Also, I notice that you used the unidiomatic
expression i != 3 in your loop condition, and I
immediately have to wonder if you are doing gymnastics
with the index inside of your loop. By
putting all the processing into separate functions,
it is clear that nothing odd is going on with i.

A good guiding principle is that if a comment is used
to describe what several lines of code are doing, then
those lines belong in a separate function. Another is
that if your closing brace is so far away from the
opening brace that you think the closing brace needs
a comment to aid in deciphering which block it closes,
then that block needs to be pruned.
Aug 19 '08 #17
santosh <sa*********@gmail.comwrites:
arnuld wrote:
[...]
>#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"

Bad idea to hardwire absolute paths like this. Better to use a relative
path.
Relative to what?

<OT>
This is somewhat Unix-specific, but a relative path is relative to the
current directory in which the program happens to be running. That
might make sense for a simple file name, but even then it's usually
better to write the log to a consistent location.

For production code, you want to get the log file name from an
external source, perhaps a configuration file (whose location also
needs to be specified) or an environment variable.
</OT>
>#define LOG_NAME "%d.log"
I'd give this a name that indicates that it's to be used as a printf
format, perhaps "LOG_NAME_FORMAT".
>enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10, LOG_CNT_MAX
= 6, LOG_BASE_NUM = 0 };

What's the advantage to using enum here. It's simply misleading.
I disagree. It's a nice trick for defining constants of type int
without using the preprocessor. But I'd put each one on its own line:

enum { PATH_SIZE = 101,
LOG_NAME_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

or maybe even:

enum { PATH_SIZE = 101 };
enum { LOG_NAME_SIZE = 8 };
enum { LOG_SIZE = 10, };
enum { LOG_CNT_MAX = 6, };
enum { LOG_BASE_NUM = 0 };

A comment explaining that you're just defining constants, not a type,
would be useful to those not familiar with the trick.

[snip]

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Aug 19 '08 #18
Keith Thompson wrote:
santosh <sa*********@gmail.comwrites:
>arnuld wrote:
[ ... ]
>>enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10,
LOG_CNT_MAX = 6, LOG_BASE_NUM = 0 };

What's the advantage to using enum here. It's simply misleading.

I disagree. It's a nice trick for defining constants of type int
without using the preprocessor. But I'd put each one on its own line:

enum { PATH_SIZE = 101,
LOG_NAME_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

or maybe even:

enum { PATH_SIZE = 101 };
enum { LOG_NAME_SIZE = 8 };
enum { LOG_SIZE = 10, };
enum { LOG_CNT_MAX = 6, };
enum { LOG_BASE_NUM = 0 };

A comment explaining that you're just defining constants, not a type,
would be useful to those not familiar with the trick.
Okay, but enum, when used like this, loses what little advantage it does
have over preprocessor #defines, IMHO. These symbolic constants are not
related and hence don't form a sensible "enumeration".

Aug 19 '08 #19
santosh <sa*********@gmail.comwrites:
Keith Thompson wrote:
>santosh <sa*********@gmail.comwrites:
>>arnuld wrote:

[ ... ]
>>>enum { PATH_SIZE = 101, LOG_NAME_SIZE = 8, LOG_SIZE = 10,
LOG_CNT_MAX = 6, LOG_BASE_NUM = 0 };

What's the advantage to using enum here. It's simply misleading.

I disagree. It's a nice trick for defining constants of type int
without using the preprocessor. But I'd put each one on its own line:

enum { PATH_SIZE = 101,
LOG_NAME_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

or maybe even:

enum { PATH_SIZE = 101 };
enum { LOG_NAME_SIZE = 8 };
enum { LOG_SIZE = 10, };
enum { LOG_CNT_MAX = 6, };
enum { LOG_BASE_NUM = 0 };

A comment explaining that you're just defining constants, not a type,
would be useful to those not familiar with the trick.

Okay, but enum, when used like this, loses what little advantage it does
have over preprocessor #defines, IMHO. These symbolic constants are not
related and hence don't form a sensible "enumeration".
It has the advantage that the identifiers are scoped and more likely
to be visible in a debugger.

They're not intended to form a sensible enumeration.

I think it's a clever and harmless little hack (in the good sense of
the word "hack") that compensates, in a limited way, for a missing
language feature.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Aug 19 '08 #20
On Tue, 19 Aug 2008 05:42:43 -0700, Nick Keighley wrote:
not bad. I might well leave it there. I'm assuming the code
actually works! I haven't compiled it, reviewed it or tested it.
If it's bug free and your Requirments Authority (which might be
you with another hat) isn't demanding any more changes then
I'd stop working on it. If it *does* have bugs or new requirements
it should be much easier to see what is going on and modify it.
My Demanding Authority (which is not me) have asked for a change. He wants
me to not to open/close the file every time we enter into the loop but
open it once and close it only when filesize hits the LOG_SIZE. I think
I need to pass 1 or 2 more parameters here and put if conditions before I
open or close the file.


--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 20 '08 #21
On Tue, 19 Aug 2008 08:31:22 -0700, William Pursell wrote:
A general suggestion--instead of writing block comments followed
by code, put the block in a function. For example, you write:
... SNIP.....
It would be much clearer of you wrote this as:
for( int i = 0; i < 3; i++ ) {
if( !log_cnt )
create_first_logfile( ... );
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
append_current_log( ... );
else
rotate_logs( ... ) ;
}

okay, I have written 1.3 versio. See at the end of the message.

but all of that should be the single line:
append_data_to_log( ... );
you mean main() should be calling only 1 function ? by that way I have to
pass 5-6 arguments to append_to_data() or better I have to make a
structure whihc has 6 members abd pass that structure to the one function
being called in main and then have to extract the values of those members
to check them in if condition. Don't you think it will complicate things ?
Also, I notice that you used the unidiomatic
expression i != 3 in your
loop condition, and I immediately have to wonder if you are doing
gymnastics with the index inside of your loop. By putting all the
processing into separate functions, it is clear that nothing odd is
going on with i.
I am sure I did not understand your intent. There is nothing happening to
index counter "i" anyway.

A good guiding principle is that if a comment is used to describe what
several lines of code are doing, then those lines belong in a separate
function. Another is that if your closing brace is so far away from the
opening brace that you think the closing brace needs a comment to aid in
deciphering which block it closes, then that block needs to be pruned.
yes, thats why I pruned the contents of for( ... ) loop:

/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.3
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
#define LOG_NAME_FORMAT "%d.log"

enum { PATH_SIZE = 101, LOG_NAME_FORMAT_SIZE = 8, LOG_SIZE = 10, LOG_CNT_MAX = 6, LOG_BASE_NUM = 0 };

void create_first_logfile( int, char*, char*, const size_t , int* );
void append_log( char*, char*, const size_t, int* );
void rotate_logs( char*, char*, const size_t, int* );

int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );

char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "UDP/IP\n";
int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_FORMAT_SIZE];
long log_size;
int log_cnt;

memset( log_name, '\0', LOG_NAME_FORMAT_SIZE);
log_size = 0;

log_cnt = oldest_log( log_path );
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );

const size_t log_recv_size = strlen( log_recv_arr );
int *const log_cnt_p = &log_cnt;

for(int i = 0 ; i != 3; ++i)
{
// SCENERIO 1
if( !log_cnt )
{
create_first_logfile( log_size, log_name, log_path, log_recv_size, log_cnt_p );
}

// SCENERIO (2)
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
{
append_log( log_name, log_path, log_recv_size, log_cnt_p );
}

// SCENERIO (3)
else // log_cnt >= LOG_CNT_MAX
{
rotate_logs( log_name, log_path, log_recv_size, log_cnt_p );
}
}

return 0;
}

// ---------------- ** Abstracted Functions ** --------------------

void rotate_logs( char* log_name, char* log_path, const size_t log_recv_size, int* log_cnt_p )
{
int log_size, log_cnt;

log_cnt = --*log_cnt_p;

rename_logs( log_name, log_path, --log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void append_log( char* log_name, char* log_path, const size_t log_recv_size, int* log_cnt_p )
{
int log_cnt, log_size;

log_cnt = *log_cnt_p;

create_logpath( log_path, log_name );
rename_logs( log_name, log_path, log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void create_first_logfile( int log_size, char *log_name, char *log_path, const size_t log_recv_size, int *log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

// it means the file does not exist
if( ! log_size )
{
log_size = create_logfile_zero( log_name, log_path, log_cnt );
}
else if( (log_size + log_recv_size) <= LOG_SIZE )
{
log_size = create_logfile( log_name, log_path );
}
else // else if( (log_size + log_recv_size) LOG_SIZE )
{
++*log_cnt_p;
strcpy(log_path, LOG_DIR);
}

}

// rename and create logs
int rename_logs( char *log_name, char *log_path, int log_cnt )
{
int log_size;

log_size = find_logsize( log_path );

rename_files( log_cnt );
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME_FORMAT );
return log_size;
}
// create logs other than 0.log
int create_logfile( char *log_name, char *log_path )
{
int log_size;

create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

// clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}

// create 0.log
int create_logfile_zero( char *log_name, char *log_path, int log_num )
{
int log_size;

create_logname( log_name, log_num );
create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

//clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}


//------ ----- ** filename and filesize related functions ** -------------------

// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME_FORMAT, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// created the full log path name
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_FORMAT_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_FORMAT_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
create_logname( log_name, i );
create_logpath( log_path, log_name );

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
}
return log_cnt;
}
// update the log count
void update_logcount( int log_size, int log_recv_size, int *log_cnt_p )
{
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++*log_cnt_p;
}
}
// -------------------- ** File I/O functions ** ---------------------------

// create a new file, write data and close the file in the end
void create_log( const char *log_path )
{
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );

if( ! (fp = fopen(log_path, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

log_size = find_logsize( log_path );

if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}

if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}

if( fclose(fp) )
{
perror("FLOSE() ERROR");
exit( EXIT_FAILURE );
}
}

// rename files
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_FORMAT_SIZE];
char temp_newname[LOG_NAME_FORMAT_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

for( int i = log_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
}
}
}

--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 20 '08 #22
On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:
1) Not in the religion of this group. Dogmatically, "C" == "C90" (and
nothing more). You'll get used to it, if you stick around long enough.
I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come, commenting
and uncommenting rest of the lines with // comments. Thats how I do
debugging.

2nd nearly all of them times, the code written by other programmers in my
team has snprintf() in it, which is C99 standard, hence I have to make
make sure of the other expressions they write and functions they create. I
make that sure by using -std=c99 and -pedantic flag. I only work on Linux
platform, so I don't have to worry much about portability but I still use
all 4 flags -std=c99. -pedantic, -Wall , -Wextra as they teach me good
programming practices.

--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 20 '08 #23
arnuld wrote:
>On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:
>1) Not in the religion of this group. Dogmatically, "C" == "C90"
(and
nothing more). You'll get used to it, if you stick around long
enough.

I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come,
commenting and uncommenting rest of the lines with // comments. Thats
how I do debugging.
Assert could be a better option than this method.

<snip>

Aug 20 '08 #24
In article <g8**********@registered.motzarella.org>,
santosh <sa*********@gmail.comwrote:
>arnuld wrote:
>>On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:
>>1) Not in the religion of this group. Dogmatically, "C" == "C90"
(and
nothing more). You'll get used to it, if you stick around long
enough.

I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come,
commenting and uncommenting rest of the lines with // comments. Thats
how I do debugging.

Assert could be a better option than this method.
??? Assert has nothing to do with what he is talking about.

Aug 20 '08 #25
On Tue, 19 Aug 2008 05:42:43 -0700, Nick Keighley wrote:
not bad. I might well leave it there. I'm assuming the code
actually works! I haven't compiled it, reviewed it or tested it.
If it's bug free and your Requirments Authority (which might be
you with another hat) isn't demanding any more changes then
I'd stop working on it. If it *does* have bugs or new requirements
it should be much easier to see what is going on and modify it.
I tried making wrapper functions for fopen(), fwrite() and flose() and
whole program has fallen to its knees, sometimes working, sometimes it
Segfaults and sometimes it overwrites beyond the allotted file size:

/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.4
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
#define LOG_NAME_FORMAT "%d.log"

enum { PATH_SIZE = 101,
LOG_NAME_FORMAT_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

void create_first_logfile( int, char*, char*, const size_t , int* );
void append_log( char*, char*, const size_t, int* );
void rotate_logs( char*, char*, const size_t, int* );

int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );

FILE* open_file( const char* );
void write_file( FILE*, const char*, const size_t );
void close_file( FILE* );
char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "Love\n";

int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_FORMAT_SIZE];
long log_size;
int log_cnt;

memset( log_name, '\0', LOG_NAME_FORMAT_SIZE);
log_size = 0;

log_cnt = oldest_log( log_path );
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );

const size_t log_recv_size = strlen( log_recv_arr );
int *const log_cnt_p = &log_cnt;

for(int i = 0 ; i != 3; ++i)
{
if( !log_cnt )
{
create_first_logfile( log_size, log_name, log_path, log_recv_size, log_cnt_p );
}
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
{
append_log( log_name, log_path, log_recv_size, log_cnt_p );
}
else // log_cnt >= LOG_CNT_MAX
{
rotate_logs( log_name, log_path, log_recv_size, log_cnt_p );
}
}

return 0;
}

// ---------------- ** Abstracted Functions ** --------------------

void rotate_logs( char* log_name, char* log_path, const size_t log_recv_size, int* log_cnt_p )
{
int log_size, log_cnt;

log_cnt = --*log_cnt_p;

rename_logs( log_name, log_path, --log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void append_log( char* log_name,
char* log_path,
const size_t log_recv_size,
int* log_cnt_p )
{
int log_cnt, log_size;

log_cnt = *log_cnt_p;

create_logpath( log_path, log_name );
rename_logs( log_name, log_path, log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void create_first_logfile( int log_size,
char *log_name,
char *log_path,
const size_t log_recv_size,
int *log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

// it means the file does not exist
if( ! log_size )
{
log_size = create_logfile_zero( log_name, log_path, log_cnt );
}
else if( (log_size + log_recv_size) <= LOG_SIZE )
{
log_size = create_logfile( log_name, log_path );
}
else // else if( (log_size + log_recv_size) LOG_SIZE )
{
++*log_cnt_p;
strcpy(log_path, LOG_DIR);
}

}
// rename all logs
int rename_logs( char *log_name, char *log_path, int log_cnt )
{
int log_size;

log_size = find_logsize( log_path );

rename_files( log_cnt );
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME_FORMAT );
return log_size;
}
// create logs other than 0.log
int create_logfile( char *log_name, char *log_path )
{
int log_size;

create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

// clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}

// create 0.log
int create_logfile_zero( char *log_name, char *log_path, int log_num )
{
int log_size;

create_logname( log_name, log_num );
create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

//clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}


//------ ----- ** filename and filesize related functions ** -------------------

// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME_FORMAT, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// created the full log path name
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_FORMAT_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_FORMAT_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
create_logname( log_name, i );
create_logpath( log_path, log_name );

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
}
return log_cnt;
}
// update the log count
void update_logcount( int log_size, int log_recv_size, int *log_cnt_p )
{
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++*log_cnt_p;
}
}
// -------------------- ** File I/O functions ** ---------------------------

// create a new file, write data and close the file in the end
void create_log( const char *log_path )
{
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );

log_size = find_logsize( log_path );
if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}
fp = open_file( log_path );
write_file( fp, log_recv_arr, log_recv_size );
close_file( fp );
}

// rename files
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_FORMAT_SIZE];
char temp_newname[LOG_NAME_FORMAT_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

for( int i = log_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
}
}
}

FILE* open_file( const char* filepath )
{
FILE* fp;

if( ! (fp = fopen(filepath, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

return fp;
}

void write_file( FILE *fp, const char* log_recv_arr, const size_t log_recv_size )
{
if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}
}
void close_file( FILE *fp )
{
if( fclose(fp) )
{
perror("FLOSE() ERROR");
exit( EXIT_FAILURE );
}
}
--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 20 '08 #26
On 2008-08-20, arnuld <su*****@invalid.addresswrote:
>On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:
>1) Not in the religion of this group. Dogmatically, "C" == "C90" (and
nothing more). You'll get used to it, if you stick around long enough.

I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come, commenting
and uncommenting rest of the lines with // comments. Thats how I do
debugging.
Better to comment out code with

#if 0
...
#endif

That way you can re-enable it easily with #if 1, or remove the conditions
entirely when you don't need it anymore. This also eliminates any problems
with commenting.
2nd nearly all of them times, the code written by other programmers in my
team has snprintf() in it, which is C99 standard, hence I have to make
make sure of the other expressions they write and functions they create. I
make that sure by using -std=c99 and -pedantic flag. I only work on Linux
platform, so I don't have to worry much about portability but I still use
all 4 flags -std=c99. -pedantic, -Wall , -Wextra as they teach me good
programming practices.
Sounds good, as long as you are aware that gcc (which I assume you are
using) is missing a few C99 features, and a few are broken.

--
Andrew Poelstra ap*******@wpsoftware.com
To email me, use the above email addresss with .com set to .net
Aug 20 '08 #27
Andrew Poelstra <ap*******@supernova.homewrites:
On 2008-08-20, arnuld <su*****@invalid.addresswrote:
>>On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:
>>1) Not in the religion of this group. Dogmatically, "C" == "C90" (and
nothing more). You'll get used to it, if you stick around long enough.

I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come, commenting
and uncommenting rest of the lines with // comments. Thats how I do
debugging.

Better to comment out code with

#if 0
...
#endif

That way you can re-enable it easily with #if 1, or remove the conditions
entirely when you don't need it anymore. This also eliminates any problems
with commenting.
[...]

Agreed, mostly. But if you comment out a large block of code with
"#if 0 ... #endif", and then look at something in the middle of the
block, it can be hard to tell at a glance that it's inactive. If you
instead comment out a large block of code using "//" comments
(assuming your compiler supports them), then it's obvious that each
line is commented out.

I've done a lot of programming in languages that *only* have
end-of-line comments ('#' in Perl, '--' in Ada), and I've found it
easy, with a decent editor and or other tools, to comment and
uncomment large blocks of code. This approach also has the advantage
of not changing line numbers.

Another option is to add "#if 0" and "#endif" before and after the
block of code *and* to add, say, a '*' character at the start of each
line. With any decent editor, adding or removing this markup should
be doable in just a few keystrokes.

(Old-style "/* */" comments can't easily be used to comment out blocks
of code, because they don't nest.)

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
Aug 20 '08 #28
On 2008-08-20, Keith Thompson <ks***@mib.orgwrote:
Andrew Poelstra <ap*******@supernova.homewrites:
>On 2008-08-20, arnuld <su*****@invalid.addresswrote:
>>>On Tue, 19 Aug 2008 13:41:48 +0000, Kenny McCormack wrote:

1) Not in the religion of this group. Dogmatically, "C" == "C90" (and
nothing more). You'll get used to it, if you stick around long enough.

I uses // because when I write code and if any error occurs, then I
comment the whole part, where I guess the error is, in /* */ style and
then 1 by 1 I explore each line from where error could come, commenting
and uncommenting rest of the lines with // comments. Thats how I do
debugging.

Better to comment out code with

#if 0
...
#endif

That way you can re-enable it easily with #if 1, or remove the conditions
entirely when you don't need it anymore. This also eliminates any problems
with commenting.
[...]

Agreed, mostly. But if you comment out a large block of code with
"#if 0 ... #endif", and then look at something in the middle of the
block, it can be hard to tell at a glance that it's inactive. If you
instead comment out a large block of code using "//" comments
(assuming your compiler supports them), then it's obvious that each
line is commented out.
Many text editors (including my own) will color #if 0...#endif
comments the same as any other comment, and most programming-based
text editors will allow you to add the tokens '#if 0' and #endif'
as comment delimeters.

This is my experience, anyway.

--
Andrew Poelstra ap*******@wpsoftware.com
To email me, use the above email addresss with .com set to .net
Aug 20 '08 #29
Andrew Poelstra wrote:
On 2008-08-20, Keith Thompson <ks***@mib.orgwrote:
<snip>
>Agreed, mostly. But if you comment out a large block of code with
"#if 0 ... #endif", and then look at something in the middle of the
block, it can be hard to tell at a glance that it's inactive. If you
instead comment out a large block of code using "//" comments
(assuming your compiler supports them), then it's obvious that each
line is commented out.

Many text editors (including my own) will color #if 0...#endif
comments the same as any other comment, and most programming-based
text editors will allow you to add the tokens '#if 0' and #endif'
as comment delimeters.

This is my experience, anyway.
Virtually every programming editor I have used renders code between #if
0 #endif in the same colour as code between /* and */ or // and EOL.

Aug 20 '08 #30
On 20 Aug, 07:24, arnuld <sunr...@invalid.addresswrote:
On Tue, 19 Aug 2008 08:31:22 -0700, William Pursell wrote:
A general suggestion--instead of writing block comments followed
by code, put the block in a function. *For example, you write:
... SNIP.....
It would be much clearer of you wrote this as:
for( int i = 0; i < 3; i++ ) {
* if( !log_cnt )
* * create_first_logfile( ... );
* else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
* * append_current_log( ... );
* else
* * rotate_logs( ... ) ;
}

okay, I have written 1.3 versio. See at the end of the message.
but all of that should be the single line:
append_data_to_log( ... );

you mean main() should be calling only 1 function ?
No, I just mean that the 8 lines above (the 3 condition
if /else clause) should be a single function call.

>
Also, I notice that you used the unidiomatic
expression i != 3 in your
loop condition, and I immediately have to wonder if you are doing
gymnastics with the index inside of your loop. *By putting all the
processing into separate functions, it is clear that nothing odd is
going on with i.

I am sure I did not understand your intent. There is nothing happening to
index counter "i" anyway.
Yes, and my point is that it is difficult to see that because
there are some 20 lines of clutter. The maintenance programmer
should not have to spend any more than 1 or 2 seconds determining
that i is not being modified, but the way you have written it,
it takes substantially more time than that to scan the code.

Aug 20 '08 #31
On 20 Aug, 18:17, Keith Thompson <ks...@mib.orgwrote:
Andrew Poelstra <apoels...@supernova.homewrites:
Better to comment out code with
#if 0
*...
#endif
That way you can re-enable it easily with #if 1, or remove the conditions
entirely when you don't need it anymore. This also eliminates any problems
with commenting.

[...]

Agreed, mostly. *But if you comment out a large block of code with
"#if 0 ... #endif", and then look at something in the middle of the
block, it can be hard to tell at a glance that it's inactive. *If you
instead comment out a large block of code using "//" comments
(assuming your compiler supports them), then it's obvious that each
line is commented out.

I've done a lot of programming in languages that *only* have
end-of-line comments ('#' in Perl, '--' in Ada), and I've found it
easy, with a decent editor and or other tools, to comment and
uncomment large blocks of code. *This approach also has the advantage
of not changing line numbers.
Another big advantage of end-of-line comments (and I say this
as someone who avoids them like the plague when writing C,
although maybe I'm starting to change that habit) is that
the comment is clear when you are looking at code selectively
(ie, via grep). Sometimes, it is convenient to do:
$ grep function_name *.c
and it is really helpful to be able to see in the output
which calls are commented.
Aug 20 '08 #32
On 2008-08-20, William Pursell <bi**********@gmail.comwrote:
>
Another big advantage of end-of-line comments (and I say this
as someone who avoids them like the plague when writing C,
although maybe I'm starting to change that habit) is that
the comment is clear when you are looking at code selectively
(ie, via grep). Sometimes, it is convenient to do:
$ grep function_name *.c
and it is really helpful to be able to see in the output
which calls are commented.
I find that code is usually only commented out temporarily
during a bug-fixing session, that I rarely, if ever, need
to grep through my codebase during that time.

At all other points in time, I don't have function calls inside
comments. Possibly I will reference the function in a block
comment, but I usually start each line of block comments with
a '*' anyway.

--
Andrew Poelstra ap*******@wpsoftware.com
To email me, use the above email addresss with .com set to .net
Aug 21 '08 #33
On Aug 20, 11:02 pm, William Pursell <bill.purs...@gmail.comwrote:
On 20 Aug, 18:17, Keith Thompson <ks...@mib.orgwrote:
<snip>
I've done a lot of programming in languages that *only* have
end-of-line comments ('#' in Perl, '--' in Ada), and I've found it
easy, with a decent editor and or other tools, to comment and
uncomment large blocks of code. This approach also has the advantage
of not changing line numbers.

Another big advantage of end-of-line comments (and I say this
as someone who avoids them like the plague when writing C,
although maybe I'm starting to change that habit) is that
the comment is clear when you are looking at code selectively
(ie, via grep). Sometimes, it is convenient to do:
$ grep function_name *.c
and it is really helpful to be able to see in the output
which calls are commented.

//\
function_name();

:-P
Aug 21 '08 #34
On Wed, 20 Aug 2008 17:34:58 +0000, Andrew Poelstra wrote:
Many text editors (including my own) will color #if 0...#endif
comments the same as any other comment, and most programming-based
text editors will allow you to add the tokens '#if 0' and #endif'
as comment delimeters.
In my case, emacs treats the part between #if 0 and #endif as
comments but I do not see any colorizing as in // or /* */ comments which
confuses a lot.

--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #35
On Wed, 20 Aug 2008 12:55:50 -0700, William Pursell wrote:

Yes, and my point is that it is difficult to see that because
there are some 20 lines of clutter. The maintenance programmer
should not have to spend any more than 1 or 2 seconds determining
that i is not being modified, but the way you have written it,
it takes substantially more time than that to scan the code.
okay, so that was the problem. I have changed it my code, it compiles file
but Segmentation Fault at run-time:
/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.4
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
#define LOG_NAME_FORMAT "%d.log"

enum { PATH_SIZE = 101,
LOG_NAME_FORMAT_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

void append_data_to_log( int, char*, char*, size_t, int* );

void create_first_logfile( int, char*, char*, const size_t , int* );
void append_log( char*, char*, const size_t, int* );
void rotate_logs( char*, char*, const size_t, int* );

int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );

void open_file( const char* );
void write_file( FILE*, const char*, const size_t );
void close_file( FILE* );
char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "Love\n";
FILE *global_fp;

int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_FORMAT_SIZE];
long log_size;
int log_cnt;

memset( log_name, '\0', LOG_NAME_FORMAT_SIZE);
log_size = 0;

log_cnt = oldest_log( log_path );
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );

const size_t log_recv_size = strlen( log_recv_arr );
int *const log_cnt_p = &log_cnt;

append_data_to_log( log_size, log_name, log_path, log_recv_size, log_cnt_p );

return 0;
}

// ---------------- ** Abstracted Functions ** --------------------

void append_data_to_log( int log_size,
char* log_name,
char* log_path,
size_t log_recv_size,
int* log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

for(int i = 0 ; i != 3; ++i)
{
if( !log_cnt )
{
create_first_logfile( log_size, log_name, log_path, log_recv_size, log_cnt_p );
}
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
{
append_log( log_name, log_path, log_recv_size, log_cnt_p );
}
else // log_cnt >= LOG_CNT_MAX
{
rotate_logs( log_name, log_path, log_recv_size, log_cnt_p );
}
}
}
void rotate_logs( char* log_name, char* log_path, const size_t log_recv_size, int* log_cnt_p )
{
int log_size, log_cnt;

log_cnt = --*log_cnt_p;

rename_logs( log_name, log_path, --log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void append_log( char* log_name,
char* log_path,
const size_t log_recv_size,
int* log_cnt_p )
{
int log_cnt, log_size;

log_cnt = *log_cnt_p;

create_logpath( log_path, log_name );
rename_logs( log_name, log_path, log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void create_first_logfile( int log_size,
char *log_name,
char *log_path,
const size_t log_recv_size,
int *log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

// it means the file does not exist
if( ! log_size )
{
log_size = create_logfile_zero( log_name, log_path, log_cnt );
}
else if( (log_size + log_recv_size) <= LOG_SIZE )
{
log_size = create_logfile( log_name, log_path );
}
else // else if( (log_size + log_recv_size) LOG_SIZE )
{
++*log_cnt_p;
strcpy(log_path, LOG_DIR);
}

}
// rename all logs
int rename_logs( char *log_name, char *log_path, int log_cnt )
{
int log_size;

log_size = find_logsize( log_path );

rename_files( log_cnt );
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME_FORMAT );
return log_size;
}
// create logs other than 0.log
int create_logfile( char *log_name, char *log_path )
{
int log_size;

create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

// clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}

// create 0.log
int create_logfile_zero( char *log_name, char *log_path, int log_num )
{
int log_size;

create_logname( log_name, log_num );
create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

//clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}


//------ ----- ** filename and filesize related functions ** -------------------

// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME_FORMAT, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// created the full log path name
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_FORMAT_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_FORMAT_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
create_logname( log_name, i );
create_logpath( log_path, log_name );

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
}
return log_cnt;
}
// update the log count
void update_logcount( int log_size, int log_recv_size, int *log_cnt_p )
{
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++*log_cnt_p;
}
}
// -------------------- ** File I/O functions ** ---------------------------

// create a new file, write data and close the file in the end
void create_log( const char *log_path )
{
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );

log_size = find_logsize( log_path );
if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}
open_file( log_path );
write_file( fp, log_recv_arr, log_recv_size );
close_file( fp );
}

// rename files
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_FORMAT_SIZE];
char temp_newname[LOG_NAME_FORMAT_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

for( int i = log_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
}
}
}

void open_file( const char * filepath )
{
FILE *fp;

if( ! (fp = fopen(filepath, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}
}
void write_file( FILE *fp, const char* log_recv_arr, const size_t log_recv_size )
{
if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}
}
void close_file( FILE *fp )
{
if( fclose(fp) )
{
perror("FLOSE() ERROR");
exit( EXIT_FAILURE );
}
}
--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #36
On Thu, 21 Aug 2008 10:05:23 +0500, arnuld wrote:
okay, so that was the problem. I have changed it my code, it compiles file
but Segmentation Fault at run-time:

I have corrected the code and removed the Segfaults. Now program runs fine
, the only problem is it does not do what is intended. It overwrites the
array beyond its alloted size:

/* The Logging program:

* A programs that will take input from stdin and put that into log files.
* using C99 (GNU-Linux/UNIX specific extensions may be there)
*
* VERSION 1.5
*
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <dirent.h>
#include <errno.h>

#define LOG_DIR "/home/arnuld/programs/ztest/CLS/"
#define LOG_NAME_FORMAT "%d.log"

enum { PATH_SIZE = 101,
LOG_NAME_FORMAT_SIZE = 8,
LOG_SIZE = 10,
LOG_CNT_MAX = 6,
LOG_BASE_NUM = 0 };

void append_data_to_log( int, char*, char*, size_t, int* );

void create_first_logfile( int, char*, char*, const size_t , int* );
void append_log( char*, char*, const size_t, int* );
void rotate_logs( char*, char*, const size_t, int* );

int create_logfile_zero( char *, char *, int );
int create_logfile( char *, char * );
int rename_logs( char *, char *, int );
void create_logname( char *, int );
void create_logpath( char*, char* );
void create_log( const char * );

int find_logsize( const char * );
int oldest_log( char * );
void rename_files( int );
void update_logcount( int, int, int* );

FILE* open_file( const char* );
void write_file( FILE*, const char*, const size_t );
void close_file( FILE* );
char log_path[PATH_SIZE] = "/home/arnuld/programs/ztest/";
const char log_recv_arr[] = "Love\n";
FILE *global_fp;

int main( void )
{
char log_path[PATH_SIZE] = LOG_DIR;
char log_name[LOG_NAME_FORMAT_SIZE];
long log_size;
int log_cnt;

memset( log_name, '\0', LOG_NAME_FORMAT_SIZE);
log_size = 0;

log_cnt = oldest_log( log_path );
create_logname( log_name, log_cnt );
log_size = find_logsize( log_name );

const size_t log_recv_size = strlen( log_recv_arr );
int *const log_cnt_p = &log_cnt;

printf("main(..) OK \n");
append_data_to_log( log_size, log_name, log_path, log_recv_size, log_cnt_p );
printf("main(..) DONE\n");

return 0;
}

// ---------------- ** Abstracted Functions ** --------------------

void append_data_to_log( int log_size,
char* log_name,
char* log_path,
size_t log_recv_size,
int* log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

for(int i = 0 ; i != 3; ++i)
{
if( !log_cnt )
{
printf("Entering ... SCENERIO 1\n\n");
create_first_logfile( log_size, log_name, log_path, log_recv_size, log_cnt_p );
printf("SCENERIO 1 ... DONE\n\n");
}
else if( (log_cnt 0) && (log_cnt < LOG_CNT_MAX) )
{
printf("Entering ... SCENERIO 2\n\n");
append_log( log_name, log_path, log_recv_size, log_cnt_p );
printf("SCENERIO 2 ... DONE\n\n");
}
else // log_cnt >= LOG_CNT_MAX
{
printf("Entering ... SCENERIO 3\n\n");
rotate_logs( log_name, log_path, log_recv_size, log_cnt_p );
printf("SCENERIO 3 ... DONE\n\n");
}
}
}
void rotate_logs( char* log_name, char* log_path, const size_t log_recv_size, int* log_cnt_p )
{
int log_size, log_cnt;

log_cnt = --*log_cnt_p;

rename_logs( log_name, log_path, --log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void append_log( char* log_name,
char* log_path,
const size_t log_recv_size,
int* log_cnt_p )
{
int log_cnt, log_size;

log_cnt = *log_cnt_p;

create_logpath( log_path, log_name );
rename_logs( log_name, log_path, log_cnt );
log_size = create_logfile_zero( log_name, log_path, LOG_BASE_NUM );

update_logcount( log_size, log_recv_size, log_cnt_p );

}

void create_first_logfile( int log_size,
char *log_name,
char *log_path,
const size_t log_recv_size,
int *log_cnt_p )
{
int log_cnt;

log_cnt = *log_cnt_p;

// it means the file does not exist
if( ! log_size )
{
printf("Entering case 1, SCENERIO 1, log_size = %d ... \n\n", log_size);
log_size = create_logfile_zero( log_name, log_path, log_cnt );
printf("Case 1, SCENERIO 1 ... DONE\n\n");
}
else if( (log_size + log_recv_size) <= LOG_SIZE )
{
printf("Entering Case 2, SCENERIO 1, log_size = %d ... \n\n", log_size);
log_size = create_logfile( log_name, log_path );
printf("Case 2, SCENERIO 1 ... DONE\n\n");
}
else // else if( (log_size + log_recv_size) LOG_SIZE )
{
printf("Entering case 3, SCENERIO 1, log_size = %d ... \n\n", log_size);
++*log_cnt_p;
strcpy(log_path, LOG_DIR);
printf("Case 3, SCENERIO 1 ... DONE\n\n");
}
}
// rename all logs
int rename_logs( char *log_name, char *log_path, int log_cnt )
{
int log_size;

log_size = find_logsize( log_path );

rename_files( log_cnt );
strcpy( log_path, LOG_DIR );
strcpy( log_name, LOG_NAME_FORMAT );
return log_size;
}
// create logs other than 0.log
int create_logfile( char *log_name, char *log_path )
{
int log_size;

create_logpath( log_path, log_name );
create_log( log_path );
log_size = find_logsize( log_path );

// clear log_path
strcpy(log_path, LOG_DIR);

return log_size;
}

// create 0.log
int create_logfile_zero( char *log_name, char *log_path, int log_num )
{
printf("Entered ----- create_logfile_zero ------\n");
int log_size;

create_logname( log_name, log_num );
printf("log_name CREATED: %s\n", log_name);

create_logpath( log_path, log_name );
printf("log_path CREATED: %s\n", log_path);

create_log( log_path );
printf("log CREATED\n");

log_size = find_logsize( log_path );

//clear log_path
strcpy(log_path, LOG_DIR);

printf("Leaving ----- create_logfile_zero -----\n");
return log_size;
}


//------ ----- ** filename and filesize related functions ** -------------------

// create only the name of log
void create_logname( char *filename, int filenum )
{
if( sprintf(filename, LOG_NAME_FORMAT, filenum) < 0 )
{
perror("SPRINTF ERROR - (BEGIN PROGRAM)");
exit( EXIT_FAILURE );
}
}
// created the full log path name
void create_logpath( char* filepath, char* filename )
{
// concatenate log_path and log_name
strcat( filepath, filename );
}

// finds the size of log
int find_logsize( const char *filename )
{
int log_size;
struct stat statbuf;

if( ! (stat(filename, &statbuf)) )
{
log_size = (long) statbuf.st_size;
}
else
{
log_size = 0;
}

return log_size;
}
// find the number of oldest log
int oldest_log( char * log_path )
{
int log_cnt;
char log_name[LOG_NAME_FORMAT_SIZE];
struct stat statbuf;

log_cnt = 0;
memset( log_name, '\0', LOG_NAME_FORMAT_SIZE );

for( int i = 0; i != LOG_CNT_MAX ; ++i )
{
create_logname( log_name, i );
create_logpath( log_path, log_name );

if( ! (stat(log_path, &statbuf)) )
{
log_cnt = i;
}
else if( ENOENT == errno )
{
// clear the log_path
strcpy(log_path, LOG_DIR);
continue;
}
else
{
perror("STAT ERROR");
exit( EXIT_FAILURE );
}

// clear the log_path
strcpy(log_path, LOG_DIR);
}
return log_cnt;
}
// update the log count
void update_logcount( int log_size, int log_recv_size, int *log_cnt_p )
{
if( (log_size + log_recv_size) >= LOG_SIZE )
{
++*log_cnt_p;
}
}
// -------------------- ** File I/O functions ** ---------------------------

// create a new file, write data and close the file in the end
void create_log( const char *log_path )
{
printf("Entered ---------- create log --------------------\n");
FILE *fp;
int log_size;
const size_t log_recv_size = strlen( log_recv_arr );

log_size = find_logsize( log_path );

printf("log_size = %d, log_recv_size = %ld, log_recv_arr = %s\n", log_size, (long)log_recv_size, log_recv_arr);

if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}
fp = open_file( log_path );
printf("file opened\n");
write_file( fp, log_recv_arr, log_recv_size );
printf("file written\n");
close_file( fp );

printf("Leaving ---------- create log --------------------\n");
}

// rename files
void rename_files( int log_cnt )
{
char temp_oldname[LOG_NAME_FORMAT_SIZE];
char temp_newname[LOG_NAME_FORMAT_SIZE];

char temp_oldpath[PATH_SIZE] = LOG_DIR;
char temp_newpath[PATH_SIZE] = LOG_DIR;

for( int i = log_cnt; (i LOG_BASE_NUM) && (i <= LOG_CNT_MAX); --i )
{
create_logname( temp_oldname, i-1 );
create_logname( temp_newname, i );
create_logpath( temp_oldpath, temp_oldname);
create_logpath( temp_newpath, temp_newname);

if( rename(temp_oldpath, temp_newpath) < 0 )
{
perror("RENAME ERROR :: S2 --case b");
exit( EXIT_FAILURE );
}
else
{
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
}
}
}

FILE* open_file( const char * filepath )
{
FILE *fp;

if( ! (fp = fopen(filepath, "a")) )
{
perror("FOPEN() EROR");
exit( EXIT_FAILURE );
}

return fp;
}
void write_file( FILE *fp, const char* log_recv_arr, const size_t log_recv_size )
{
if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
{
perror("FWRITE() ERROR");
exit( EXIT_FAILURE );
}
}
void close_file( FILE *fp )
{
if( fclose(fp) )
{
perror("FLOSE() ERROR");
exit( EXIT_FAILURE );
}
}

--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #37
On 21 Aug, 07:01, arnuld <sunr...@invalid.addresswrote:

I haven't looked through your code carefully, but a
quick perusal brings some thoughts to mind:
* if( log_size LOG_SIZE )
* * {
* * * perror("How could that happen ?");
* * * perror("You got buggy code ??");
* * * exit( EXIT_FAILURE );
* * }
This is exactly the right place for an assertion.
Replace the 6 lines above with:
assert( log_size LOG_SIZE );

* fp = open_file( log_path );
* printf("file opened\n");
This feels like a debugging printf. It would
be helpful if it were more obvious that that
is true, perhaps using a macro. Also, it
would help to include the line number. Perhaps

DBG( fprintf( stderr, "%s:%d: %s opened\n",
__FILE__, __LINE__, log_path ));

where you define something like:
#if DEBUG
#define DBG(x) x
#else
#define DBG(x)
#endif

(Usually, the printf and the __FILE__ and __LINE__
go in the macro, so you just do
DBG( "%s opened", log_path );
examples of how to do this are easy to find,
and I believe at least one appears elsethread.)
>
* if( ! (fp = fopen(filepath, "a")) )
* * {
* * * perror("FOPEN() EROR");
* * * exit( EXIT_FAILURE );
* * }
This code leads to one of the most annoying
error messages ever:
FOPEN() EROR: Permission denied

make the call to perror simpler:
perror( filepath );

(You may get people suggesting that a more complicated
function is necessary due to the fact that the
C standard does not require fopen to set errno,
and although this is technically true it is
probably not worth worrying about at this point. If
you are worried about it, you can do:
#ifdef FOPEN_SETS_ERRNO
perror( filepath );
#else
fprintf( stderr, "Unable to open %s: unknown reason\n"
filepath );
#endif
or something like that. Since this is just an
exercise, I recommend against this for this program.)

void write_file( FILE *fp, const char* log_recv_arr, const size_t log_recv_size )
{
* if( (fwrite(log_recv_arr, 1, log_recv_size, fp)) != log_recv_size )
* * {
* * * perror("FWRITE() ERROR");
* * * exit( EXIT_FAILURE );
* * }
Here, it is more difficult to get a useful error message,
because you only have a FILE *, and the pathname used
to open that file is lost. There are ways to deal with this
gracefully, and it is a good exercise to come up with
one(*). Error messages are really, *really* important,
and knowing the path to the file which gets
a write/close error can save you a lot of time
fixing the problem.

(*) two ideas for tracking paths:
1) define your own type:
struct myfile {
FILE *fp;
char *path;
};
and associated wrapper functions for fopen that store
the path.

2) keep an array of pathnames available, and record
the pathname used to open each file. If you're on
a POSIX system, you can use the file descriptor
of the FILE * as the index.
Aug 21 '08 #38
In article <0e**********************************@p25g2000hsf. googlegroups.com>,
William Pursell <bi**********@gmail.comwrote:
>On 21 Aug, 07:01, arnuld <sunr...@invalid.addresswrote:
>* if( log_size LOG_SIZE )
* * {
* * * perror("How could that happen ?");
* * * perror("You got buggy code ??");
* * * exit( EXIT_FAILURE );
* * }

This is exactly the right place for an assertion.
Replace the 6 lines above with:
assert( log_size LOG_SIZE );
assert(logsize <= LOG_SIZE);

Aug 21 '08 #39
In article <12****************@proxy00.news.clara.net>,
Ike Naar <ik*@localhost.claranet.nlwrote:
>In article <0e**********************************@p25g2000hsf. googlegroups.com>,
William Pursell <bi**********@gmail.comwrote:
>>assert( log_size LOG_SIZE );

assert(logsize <= LOG_SIZE);
fixing the underscore:

assert(log_size <= LOG_SIZE);
Aug 21 '08 #40
On 21 Aug, 08:00, i...@localhost.claranet.nl (Ike Naar) wrote:
In article <1219301762.2504...@proxy00.news.clara.net>,

Ike Naar <i...@localhost.claranet.nlwrote:
In article <0ede4309-8666-4930-9158-9f7bd167b...@p25g2000hsf.googlegroups.com>,
William Pursell *<bill.purs...@gmail.comwrote:
>assert( log_size LOG_SIZE );
assert(logsize <= LOG_SIZE);

fixing the underscore:

assert(log_size <= LOG_SIZE);
Yes, my mistake. (or, of course
assert( ! log_size LOG_SIZE );
whichever you prefer.)
Aug 21 '08 #41
On Thu, 21 Aug 2008 07:56:02 +0100, Ike Naar wrote:
>William Pursell <bi**********@gmail.comwrote:
>>>On 21 Aug, 07:01, arnuld <sunr...@invalid.addresswrote:
>>Â* if( log_size LOG_SIZE )
Â* Â* {
Â* Â* Â* perror("How could that happen ?");
Â* Â* Â* perror("You got buggy code ??");
Â* Â* Â* exit( EXIT_FAILURE );
Â* Â* }
>>Replace the 6 lines above with: assert( log_size LOG_SIZE );
assert(logsize <= LOG_SIZE);

???

Ike.... what happened ? you reversed the operator.
--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #42
On Wed, 20 Aug 2008 23:44:30 -0700, William Pursell wrote:
This is exactly the right place for an assertion.
Replace the 6 lines above with:
assert( log_size LOG_SIZE );
Seems like a very good thing.. code replaced :)

This feels like a debugging printf. It would
be helpful if it were more obvious that that
is true, perhaps using a macro. Also, it
would help to include the line number. Perhaps
yes, Andrew mentioned it. To me, Macros are the biggest annoying thing in
C. I avoid them all the time, except when I am helpless. They cheat the
compiler all the way. Regarding fopen(..) it is mentioned only at one
place in the source and hence if an error occurs, it tells the name of
the function and hence it is easy to know where it is. Same for all other
functions.

This code leads to one of the most annoying
error messages ever:
FOPEN() EROR: Permission denied
:D

make the call to perror simpler:
perror( filepath );
filepath is a macro, which is LOG_DIR. I have commented the
whole design of the code ( which is stripped off to make things
short in here). So any programmer who will read the code will know
everything from the comments.

(*) two ideas for tracking paths:
1) define your own type:
struct myfile {
FILE *fp;
char *path;
};
and associated wrapper functions for fopen that store the path.

2) keep an array of pathnames available, and record the pathname used to
open each file. If you're on a POSIX system, you can use the file
descriptor of the FILE * as the index.
Why go through all the this complicatedness ? See my answer above. I am
not disqualifying your code, I am asking for why put 50 more lines of
code when perror and exit do the job.


--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #43
William Pursell said:
On 21 Aug, 08:00, i...@localhost.claranet.nl (Ike Naar) wrote:
>In article <1219301762.2504...@proxy00.news.clara.net>,

Ike Naar <i...@localhost.claranet.nlwrote:
>In article
<0ede4309-8666-4930-9158-9f7bd167b...@p25g2000hsf.googlegroups.com>,
William Pursell <bill.purs...@gmail.comwrote:
assert( log_size LOG_SIZE );
>assert(logsize <= LOG_SIZE);

fixing the underscore:

assert(log_size <= LOG_SIZE);

Yes, my mistake. (or, of course
assert( ! log_size LOG_SIZE );
whichever you prefer.)
Yes, your mistake again. :-) You meant, of course,

assert(! (log_size LOG_SIZE));

The parentheses matter, because the precedence of ! is practically off the
scale, whereas is far more relaxed.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Aug 21 '08 #44
On Thu, 21 Aug 2008 08:00:51 +0100, Ike Naar wrote:
fixing the underscore:

assert(log_size <= LOG_SIZE);
Eh... and I thought assert check for wrongness.. oopssy!

--
www.lispmachine.wordpress.com
my email is @ the above blog
check the "About Myself" page

Aug 21 '08 #45
arnuld said:
>On Thu, 21 Aug 2008 07:56:02 +0100, Ike Naar wrote:
>>William Pursell <bi**********@gmail.comwrote:
On 21 Aug, 07:01, arnuld <sunr...@invalid.addresswrote:
>>>if( log_size LOG_SIZE )
{
perror("How could that happen ?");
perror("You got buggy code ??");
exit( EXIT_FAILURE );
}
>>>Replace the 6 lines above with: assert( log_size LOG_SIZE );
>assert(logsize <= LOG_SIZE);


???

Ike.... what happened ? you reversed the operator.
An assertion is a declaration of belief by the programmer that a given
condition is bound to be true. If the condition is *not* true, the
universe is unstable and must be brought to a stop so that the programmer
can lie down on his little skateboard and wheel himself back under the
engine to find out what's going wrong while the apprentice keeps him
supplied with spanners (wrenches) and coffee.

Now, in your code, you have this logic:

iff(condition) stop;

where "iff" is a short way of saying "if and only if". But assertions work
more like this:

iff(condition) keep going;

which is equivalent to

iff(!condition) stop;

So the sense of the condition itself needs to be reversed, compared to the
original. Let's plug in your condition. You had:

if(log_size LOG_SIZE) stop.

So the equivalent assertion is akin to:

if(log_size <= LOG_SIZE) keep going;

HTH. HAND.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Aug 21 '08 #46
arnuld said:
>On Wed, 20 Aug 2008 17:34:58 +0000, Andrew Poelstra wrote:
>Many text editors (including my own) will color #if 0...#endif
comments the same as any other comment, and most programming-based
text editors will allow you to add the tokens '#if 0' and #endif'
as comment delimeters.

In my case, emacs treats the part between #if 0 and #endif as
comments but I do not see any colorizing as in // or /* */ comments which
confuses a lot.
Use vim, then - quite apart from getting this right for many years now, it
also has the virtue of being usable by people with only two hands.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Aug 21 '08 #47
Richard Heathfield <rj*@see.sig.invalidwrites:
arnuld said:
>>On Wed, 20 Aug 2008 17:34:58 +0000, Andrew Poelstra wrote:
>>Many text editors (including my own) will color #if 0...#endif
comments the same as any other comment, and most programming-based
text editors will allow you to add the tokens '#if 0' and #endif'
as comment delimeters.

In my case, emacs treats the part between #if 0 and #endif as
comments but I do not see any colorizing as in // or /* */ comments which
confuses a lot.

Use vim, then - quite apart from getting this right for many years now, it
also has the virtue of being usable by people with only two hands.
You would not be trying to start an off-topic editor war would you?

I suspect the arnuld means that inside the #if 0 the syntax
highlighting is off -- overridden by that of the #if. This is the
default in vim as well as in emacs. I am sure both editors are
capable sufficient control to get user-controlled mixture of
highlighting if that is what is required.

--
Ben.
Aug 21 '08 #48
Ben Bacarisse said:
Richard Heathfield <rj*@see.sig.invalidwrites:
>arnuld said:
<snip>
>>>
In my case, emacs treats the part between #if 0 and #endif as
comments but I do not see any colorizing as in // or /* */ comments
which confuses a lot.

Use vim, then - quite apart from getting this right for many years now,
it also has the virtue of being usable by people with only two hands.

You would not be trying to start an off-topic editor war would you?
Who, moi? ;-)

Sorry, I just have this blind spot where emacs is concerned. I just can't
get the Ctrl-Meta-LeftShift-hang of it.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Aug 21 '08 #49
Ben Bacarisse <be********@bsb.me.ukwrote:
Richard Heathfield <rj*@see.sig.invalidwrites:
arnuld said:
In my case, emacs treats the part between #if 0 and #endif as
comments but I do not see any colorizing as in // or /* */ comments which
confuses a lot.
Use vim, then - quite apart from getting this right for many years now, it
also has the virtue of being usable by people with only two hands.

You would not be trying to start an off-topic editor war would you?
No, he's trying to start an off-topic vi-vs-emacs war. Editors don't
come into it.

Richard
Aug 21 '08 #50

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

20
by: Steve Jorgensen | last post by:
A while back, I started boning up on Software Engineering best practices and learning about Agile programming. In the process, I've become much more committed to removing duplication in code at a...
0
by: ¿ Mahesh Kumar | last post by:
I have created a XML file as datasource which i 'm binding to a grid view control. During runtime I'm capturing items in XML file and displaying in a GRID VIEW control. but i want to remove or...
0
by: DolphinDB | last post by:
Tired of spending countless mintues downsampling your data? Look no further! In this article, you’ll learn how to efficiently downsample 6.48 billion high-frequency records to 61 million...
0
by: ryjfgjl | last post by:
ExcelToDatabase: batch import excel into database automatically...
0
isladogs
by: isladogs | last post by:
The next Access Europe meeting will be on Wednesday 6 Mar 2024 starting at 18:00 UK time (6PM UTC) and finishing at about 19:15 (7.15PM). In this month's session, we are pleased to welcome back...
0
by: Vimpel783 | last post by:
Hello! Guys, I found this code on the Internet, but I need to modify it a little. It works well, the problem is this: Data is sent from only one cell, in this case B5, but it is necessary that data...
0
by: jfyes | last post by:
As a hardware engineer, after seeing that CEIWEI recently released a new tool for Modbus RTU Over TCP/UDP filtering and monitoring, I actively went to its official website to take a look. It turned...
1
by: PapaRatzi | last post by:
Hello, I am teaching myself MS Access forms design and Visual Basic. I've created a table to capture a list of Top 30 singles and forms to capture new entries. The final step is a form (unbound)...
1
by: Defcon1945 | last post by:
I'm trying to learn Python using Pycharm but import shutil doesn't work
1
by: Shællîpôpï 09 | last post by:
If u are using a keypad phone, how do u turn on JavaScript, to access features like WhatsApp, Facebook, Instagram....
0
isladogs
by: isladogs | last post by:
The next Access Europe User Group meeting will be on Wednesday 3 Apr 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM). In this session, we are pleased to welcome former...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.