how to remove code duplication

[arnuld]

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 <%s> to <%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 <%s> to <%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
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

 

[Nick Keighley]

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.

 

[Andrew Poelstra]

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!

 

[CBFalconer]

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.

 

[Nick Keighley]

arnuld wrote:


Throw it away.  Write simple code, with short functions.

yes, that's perhaps what I should have said.

<snip>

 

[arnuld]

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.

 

[arnuld]

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 <%s> to <%s>\n", temp_oldname, temp_newname);
// clear pathnames
strcpy(temp_oldpath, LOG_DIR);
strcpy(temp_newpath, LOG_DIR);
//sleep(2);
}
}


}

 

[santosh]

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.)

 

[Nick Keighley]

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.

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

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]

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.



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


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



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.


yes, but you didn't write any functions of your own.

Sorry. This is untrue. You did write some functions of your own.

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.

 

[Nick Keighley]

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.

 

[arnuld]

... 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);
}
}
}

 

[Ben Bacarisse]

arnuld said:


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.

 

[Richard Bos]

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

 

[Ben Bacarisse]

No, he's trying to start an off-topic vi-vs-emacs war. Editors don't
come into it.

How so? You would not be trying to start an off-topic language war
would you?

editor, n.
...
6. Computing. A program that permits the user to alter programs or
to alter or rearrange data or text held in a computer.

[OED Second Edition 1989]

 

[Richard Bos]

How so? You would not be trying to start an off-topic language war
would you?

editor, n.
...
6. Computing. A program that permits the user to alter programs or
to alter or rearrange data or text held in a computer.

vi, pr.n.
A computer program designed to stress-test the use of modal bleeping.

emacs, pr.n.
An inferior operating system noted for the lack of a proper editor.

Richard

 

[Keith Thompson]

[arnuld, you added an extra "> " on the above attribution line.
Don't do that.]

[...]

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.

Macros, when used very carefully, can be extraordinarily powerful and
useful. In this case, they can let you have two versions of your
program (one with debugging, one without it) without having to
maintain two separate copies of your sources.

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.

Sure, any programmer who reads the code will be able to figure out
where the problem is. But any *user* of the code won't have a clue.
And even as a programmer, I'd much rather have an error message that
tells me the name of the file that couldn't be opened, so I don't have
to go diving into the source code (assuming I have it) to figure out
what the problem is. Maybe I can just fix the permissions on that
file and try again.

(If the file is, say, a configuration file that should have been
correctly installed with the application, then a more general error
message that doesn't refer to the file name might be ok.)

[...]

 

[arnuld]

And where in the 400+ lines of code I just snipped did you bother to
tell us which array it overflowed? Are we supposed to guess?

hmm... okay.... my mistake.

Your insistence on using unnecessary C99 features (definitions
following executable statements, definitions within the for statement)
makes it difficult for anyone without one to help.

Seems like I will be respected better if I use C90 on comp.lang.c ?


BTW, I have already written the code and worked on it for more than a
month now and hence I don't intend to convert it to C90. The future
programs I will write after this program is done, can be done in C90 and I
still don't understand the importance of getting the local index counter
of for(...) loop into the global namespace .

 

[arnuld]

And where in the 400+ lines of code I just snipped did you bother to
tell us which array it overflowed? Are we supposed to guess?

okay, I can't find the reason except that log_cnt is not getting
incremented but don't know why.


I am throwing away the whole program and rewriting it from scratch, see my
new post titled: A File I/O Program, for my new code.

 

[Barry Schwarz]

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:

And where in the 400+ lines of code I just snipped did you bother to
tell us which array it overflowed? Are we supposed to guess?

Your insistence on using unnecessary C99 features (definitions
following executable statements, definitions within the for statement)
makes it difficult for anyone without one to help.