I am slowly working on my "final exam" for my self study in c, adn I
have found a question I can not answer.  What are the circumstance when
I should use malloc to allocate space for my variable?  Should I malloc
all of them or just strings?  Or is it based more on the memory usage of
the program and tring to minimize its footprint in the available memory?

I have looked through my books and the faq and have found a great deal
of information about how to use it but very little on when to use it.

tia
stevet

when you *don't know* how much space you will need.

the whole point of malloc is that you're able to grab more and more
space if you need it.

---
"... What with you being his parents and all, I think that you could
be trusted not to shaft him."  -- Robert Chang, rec.games.board

But if you know that you need around 10 to 12KB of memory for some temporary
storage, do you - unsigned char foo[1024*12]; and leave a 12KB footprint in
the compiled program or do you - unsigned char *foo; and malloc what you
need when you need it? Isn't it better to use malloc in this case since the
memory isn't needed all the times and the executable program get smaller?

Do you know that the buffer won't ever need to be more than 12
kB?  Is the buffer needed throughout the lifetime of the program?
Does your system omit all-bits-zero bytes from the on-disk image,
as do many?

If the answer to two or three of these is `yes', I would probably
statically allocate the buffer.

If both of those are actually true then I'd dynamically allocate
the buffer.

Some how I thought that might be what the answer would be.  So here is my
followup question, can you use malloc() to protect a variable from being
changed.  The reason I ask is I am getting a seg fault in the program I am
working on.  The value of the variable is fine until I return from the
module then it suddenly vanishes.  I have tried to follow it in dbg.  It
reports that the variable is now out of bounds.  I am not changing the value
or anything else I can think of that would give me the error.  I realize
that the included program probably has a lot of issues, but right now the
most important one to me is the return from the function reporter() back to
the function startprogram().   That is where the seg fault is originating.

I apologize for the length of the post, but as is often said include the
source code.

Instructions
1.  Cut the files out of this post and place them into a directory.
2.  Compile timeclock.c
3.  Run the resulting executable.
4.  Follow the prompts, you should use user id AS4 and use option 3 Use
Reporting functions.  When asked for the file name give it the filename
included below "tc200021.tcd".
5.  The seg fault occurs when the program returns from startprogram() to
main(), though the variable disappears when reporter() returns to
startprogram().

tia
stevet

timeclock.h

/***************************************************************************

                                 timecard.h
               A small program to collect timecard information
                             -------------------
    begin                : Thu Feb 24 2000
    copyright            : (C) 2000 by Stephen Toothman

 ***************************************************************************/

/***************************************************************************

 *                                                                         *

 *   This program is free software; you can redistribute it and/or modify  *

 *   it under the terms of the GNU General Public License as published by  *

 *   the Free Software Foundation; either version 2 of the License, or     *

 *   (at your option) any later version.                                   *

 *                                                                         *

 ***************************************************************************/

/*DEFINE list*/

/* User defined variable types */
typedef struct {
        char userid[4];     /* the user id or employee number */
        int hour;           /* time portion - hours */
        int minute;         /* time portion - minutes */
        int second;
        int month;          /* date portion - month */
        int day;
        int year;
        int action;         /* holds the action being taken */

typedef struct {
        int month;          /* date portion - month */
        int day;
        int year;

/* Function List*/
int initializeprogram(char *puserid, int *pusertype, int *pweekends);
int getenvironment(int *pweekends);
int getuserid(char *puserid, int *pusertype);
int startprogram(char userid2[4], int usertype2, int weekends2);

int timecard(char userid3[4], int usertype3, int weekends3);
int gettimeanddate(timecardrecord *currentpunch, char *pweekstring, char
*pyearstring, int weekends4);
int getfilename(char *pfilename, char *pweekstring, char *pyearstring);
int gettimecarddata(timecardrecord *currentpunch, timecardrecord
*clockpunch, int *pitemflag, char *pfilename);
int printtimecarddata(timecardrecord *currentpunch, timecardrecord
*clockpunch);
int getaction(timecardrecord *currentpunch, timecardrecord *clockpunch, int
*pitemflag);
int writerecord(timecardrecord *currentpunch, char *pfilename);

int manager(char userid3[4], int usertype3, int weekends3);

int reporter(int weekends3);
int usefilename(char *pusefile);
int getreportdates(char usefile[13], daterecord *pfiledates, int weekends4);

int printhoursreport(char *pusefile, daterecord filedates[7]);
int hoursfilename(char usefile[13], char *phoursreportfile);
int generatehoursreport(char usefile[13], char hoursreportfile[13],
daterecord filedates[7]);
int printtimecardreport(char *pusefile, daterecord filedates[7]);
int timecardfilename(char usefile[13], char *ptimecardreportfile);
int generatetimecardreport(char usefile[13], char timecardreportfile[13],
daterecord filedates[7]);

int administrator(char userid3[4], int usertype3, int weekends3);

timeclock.c

/***************************************************************************

                                     Timeclock
                 A small program to collect timecard information
                            -------------------
    begin                : Tue Feb 22 09:41:09 CST 2000
    copyright            : (C) 2000 by Stephen Toothman

 ***************************************************************************/

/***************************************************************************

 *                                                                         *

 *   This program is free software; you can redistribute it and/or modify  *

 *   it under the terms of the GNU General Public License as published by  *

 *   the Free Software Foundation; either version 2 of the License, or     *

 *   (at your option) any later version.                                   *

 *                                                                         *

 ***************************************************************************/

/***************************************************************************

 *                                                                         *

 *   This is a complete GPL time clock system for small business.  It will *

 *   have a time clock, a reporting system, an user management system, and *

 *   an administrative system for setting system options.  The timeclock   *

 *   section will rely on several external files when it is finished.      *

 *   These files will be an administration file that holds the basic       *

 *   company information, such as when the pay week begins and when the    *

 *   pay week ends and in future versions may allow different payroll      *

 *   systems such as bi-monthly and monthly.  The second file will hold a  *

 *   list of user ids and a flag for the privileges allowed individual     *

 *   user.  The last file will be the timecard file that holds the         *

 *   individual timecard records.                                          *

 *                                                                         *

 ***************************************************************************/

#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <ctype.h>
#include <string.h>
#include "timeclock.h"

int main(int argc, char *argv[])
{
        int i, j;
        char userid[4] = "zzz";     /* the user id or employee number */
        int usertype = 0;      /* the user type used to determine access */
        int weekends = 0;

        i = initializeprogram(&userid[0], &usertype, &weekends);
        j = startprogram(userid, usertype, weekends);
        printf("program ends\n");
        return EXIT_SUCCESS;

int initializeprogram(char *puserid1, int *pusertype1, int *pweekends1)
{
        int m, n, loop;
        char tempuser[4];
        int tempusertype, tempweekends;

        m = getenvironment(&tempweekends);
        n = getuserid(&tempuser[0], &tempusertype);
  for( loop = 0; loop < 4; loop++)
  {
                *puserid1 = toupper((unsigned char) tempuser[loop]);
                puserid1 = puserid1 + 1;
  }
        *pusertype1 = tempusertype;
        *pweekends1 = tempweekends;
        return n;

int getenvironment(int *pweekends2)
{
        FILE *fp;
        char fieldname[80];
        int testvalue;
        fp = fopen("admin.txt", "r");
        while ( !feof(fp))
        {
                fscanf(fp, "%s %02d\n", fieldname, &testvalue);
                if (strcmp(fieldname, "WEEKENDS") == 0)
                {
                        *pweekends2 = testvalue;
                }
        }
        fclose(fp);
        return 0;
int getuserid(char *puserid2, int *pusertype2)
{
        char *z;
        char name[80];
        char firstname[80];
        char lastname[80];
  int w = 0, x = 0, y, loop;
        FILE *fp;
        char testuserid[4];
        int testrights;

        while (x == 0)
        {
        printf("Enter your user id\n");
          z = fgets(name, 80, stdin);
          y = (z == NULL ? 0 : 1);
          for( loop = 0; loop < 4; loop++)
          {
                if (loop == 3 )
                {
                        *puserid2 = '\0';
                }
                else
                {
                        *puserid2 = toupper((unsigned char) name[loop]);
                        puserid2 = puserid2 + 1;
                }
          }
          puserid2 = puserid2 - loop + 1;
                fp = fopen("userlist.txt", "r");
                while ( !feof(fp) && x == 0)
                {
                        fscanf(fp, "%s %02d %s %s\n", testuserid,
&testrights, firstname, lastname);
...

read more »

No.

[...]

The idea is that you should include the *relevant* source code,
not every bit of source code in your program.  Certainly not 1500
lines worth.

You have to follow the same process to find your bug that we
would.  The idea is to narrow down the places where the bug can
be, by removing code until the bug disappears.  You can do this
in a "binary search" way sometimes - i.e., remove half the code,
is the bug still there?  Yes?  What if you remove the other half?
Not there anymore?  Then try removing half of *that* half, and so
on.  Now obviously code interdependencies prevent this sort of
strategy, but "unit test" is still a good idea.

Boy, that was a rambling paragraph.  My point is that looking for
bugs is a matter of figuring out where they *can't* be as much as
where they are.

As I said in my post, I know where the bug is occuring, what I can not figure
out is why?   They values remain correct until the program returns from the
reporting section.  But after the return the values are changed to undefined
values.  I do not see any reason for the value to change, so I thought the
problem might be buried somewhere in the code and I am missing it.

The only hint I can seem to find is that the variable usertype seems to equal
in value to the variable weekends after the return.  Though I am not certain
that is is the case, it often appears that way.  However other values have
shown up as well.

(Note: I have not looked at any of the rest of this code.)

These lines are virtually guaranteed to be wrong.

The feof() function tells you whether a PREVIOUS read that ALREADY
FAILED, did so due to end of file.  An feof() that appears in a
"while" or "for" loop is almost always being used to attempt to
predict whether the user is going to press ^D or ^Z, before the
user does so.  Any such prediction is probably going to be wrong.
--
In-Real-Life: Chris Torek, Berkeley Software Design Inc

!

#define SOT(_) ( sizeof(_)/sizeof(_[0]) )
for(loop=0; loop < SOT(tempuser); loop++)

if(NULL == fp) { /* ??? */ }

if(2 != fscanf(fp, "%79s %02d%*[^\n]\n", &fieldname[0], &testvalue)) { /*
Error code */ }

%d\n",
fprintf(stderr, ....); /* All interactive conversation too */

What if(feof(stdin)) ?
What if there was no digits ?
I thing that it is better to start with some preset values
and change only the ones that are expilitly named in input data stream.

--
Aleksander.

I am simply looking for the end of the file with these statements, but if
there is a better way to code stepping through a file looking for data
until the program reaches the end I would love to see it.  It seems to me
that what I have coded might be a little clunky.  I am pulling together
several sources of information in putting these functions together.  The
read and write to a file, I put together several months ago, and have not
really looked for a method to improve, since it worked.

I would suppose you are talking about searching for any type of error, not
just end of file.  So I could write the code up something like this:

read line
while (line != NULL)
{
    /* do stuff */
    read next line

stevet

I was looking for some way to make that adjustment in my program and #define
completely escaped me.  Though what I would like to do at some point is move
the user id from a fixed length to a variable length array, which is set by
a value in the administration file.  But this makes a good solution until
then.  Thank you.

This might be the same as the other post I looked at, but what you are
saying is I should not look for EOF but instead should look for the pointer
or a variable to be NULL, correct?

I am afraid I am not following you here, I understand that it tests the
return code from fscanf() for an error code, but I am not sure about how you
have it set up, particularly the part "%*[^\n]"

This means I should send all of these conversations not only to standard
out  but also to standard error as well, correct?  If so that is something
that had never crossed my mind.   I would suppose that it would make finding
what a user did wrong much easier.

The  error checking for all of those cases is handled below this section,
where I test for what is and is not a valid response given the class of user
using the program, the usertype variable.

Thank you for your help, I appreciate it.  A couple of things you mentioned
I am putting into place right away.

stevet

The problem is, you are looking behind you when you think you are
looking in front of you.  The feof() function tests for *old*,
previously-failing read operations.  It says nothing about what
will happen on the *next* operation.

Well, no and yes -- that is, there are many possible reasons for a
read operation to fail, only one of them being end-of-file.  In any
case:

Actually, what you want is:

        read a line
        while (the last read worked) {
                do something with the stuff we just read
                read another line
        }

but this can be coded as:

        while (fgets(buf, sizeof buf, fp) != NULL) {
                do something with buf[]
        }

or (for cases where fscanf suffices; but see the FAQ):

        while (fscanf(fp, fmt, p1, p2, ..., pn) == N) {
                do something with the items read in
        }

The functions that do a "read" operation generally return a
success-or-failure indication.  For fgets(), the return value is
NULL on failure; for fscanf(), the return value is less than the
number of conversions requested on matching or input failures, and
possibly less than zero (equal to EOF) on input failure.  (If the
input failure occurs after M successful conversions, fscanf() cannot
tell you both that there were M successful conversions *and* that
there was an input failure, so it simply returns M.)

When the read fails (for whatever reason), you can then use feof()
to detect whether the failure was the "expected" and "normal" EOF
case, or whether there was something more serious, such as a bad
sector on your disk (ferror()).  Most programs simply assume that
input only ever fails due to EOF, and most runtime C systems report
disk read errors separately.  (Note the chicken-and-egg question
here:  Which came first, the utilities that ignore failing disk
drives, or the error messages from the operating system?  The former
requires the latter, while the latter enables the former.  The OS
must report the error, because most of the programs do not; most
programs do not report the error, because the OS does.  But maybe
both the programs AND the OS should report it, "just in case".)
Newsgroups: comp.lang.c
Subject: Re: to malloc() or not to malloc(), that is the question

Organization: none of the above

The problem is, you are looking behind you when you think you are
looking in front of you.  The feof() function tests for *old*,
previously-failing read operations.  It says nothing about what
will happen on the *next* operation.

Well, no and yes -- that is, there are many possible reasons for a
read operation to fail, only one of them being end-of-file.  In any
case:

Actually, what you want is:

        read a line
        while (the last read worked) {
                do something with the stuff we just read
                read another line
        }

but this can be coded as:

        while (fgets(buf, sizeof buf, fp) != NULL) {
                do something with buf[]
        }

or (for cases where fscanf suffices; but see the FAQ):

        while (fscanf(fp, fmt, p1, p2, ..., pn) == N) {
                do something with the items read in
        }

The functions that do a "read" operation generally return a
success-or-failure indication.  For fgets(), the return value is
NULL on failure; for fscanf(), the return value is less than the
number of conversions requested on matching or input failures, and
possibly less than zero (equal to EOF) on input failure.  (If the
input failure occurs after M successful conversions, fscanf() cannot
tell you both that there were M successful conversions *and* that
there was an input failure, so it simply returns M.)

When the read fails (for whatever reason), you can then use feof()
to detect whether the failure was the "expected" and "normal" EOF
case, or whether there was something more serious, such as a bad
sector on your disk (ferror()).  Most programs simply assume that
input only ever fails due to EOF, and most runtime C systems report
disk read errors separately.  (Note the chicken-and-egg question
here:  Which came first, the utilities that ignore failing disk
drives, or the error messages from the operating system?  The former
requires the latter, while the latter enables the former.  The OS
must report the error, because most of the programs do not; most
programs do not report the error, because the OS does.  But maybe
both the programs AND the OS should report it, "just in case".)
--
In-Real-Life: Chris Torek, Berkeley Software Design Inc

Here is what I came up with, after spending the morning using LCLint on the code.   Does this look more like what you were talking about.

        if ((fp = fopen("admin.txt", "r")) != NULL)
        {
                while (fscanf(fp, "%s %02d\n", fieldname, &testvalue) != NULL)
                {
                        /* do something  */
                }
                (void)fclose(fp);
        }
        else
        {
                /* give error message and recover or exit */
        }

Thank You  for your help so far
 stevet

Clarification

It has to be EOF and not NULL otherwise you get a compiler error.

        if ((fp = fopen("admin.txt", "r")) != NULL)
        {
                while (fscanf(fp, "%s %02d\n", fieldname, &testvalue) != EOF)
                {
                        /* do something  */
                }
                (void)fclose(fp);
        }
        else
        {
                /* give error message and recover or exit */
        }

Thank You  for your help so far
 stevet