Very simple Unix Prog in C, could use advice/help

I just want to get the read() system call to do it's thing! LOL It's now 3:30 AM in the morning, and as I'm the stubborn type I don't like to go to bed with unsolved code :slight_smile: However I submit humbly now to anyone who can show me what my dumb mistake is. The program imitates a daemon process that so far checks to see if a specific file exists, if not then it creates it. It then locks the file and attempts to read the file while locked. If my variable fileLength is any other value than 0, the program will lock and you have to CTRL-C out. If anyone wants to help a newbie learn the ways of Unix Network Programming, please enlighten me. Here is the code, It's commented well and easy to understand, everything works except read(), which I want to read the whole file, beginning to end. Thanks to anyone who cares :):

//Program: fileinfodaemon
// By Matt Johnson

/************** INCLUDES *******************/

#include <stdlib.h>
#include <stdio.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h> //We need this for finding out statistics
//of files
#include <unistd.h> //We need this for our locking functions

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

/************ DEFINES ********************/

#define FILENAME "shared2.txt"
#define MAXBUFF (1024)

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

/******** VARIABLE DECLARATION ****************/

int fd; //File Descriptor
int loopFlag = 0; //Loop condition for while loop
int n; //Index for buff[] array
int bytesRead = 0;

struct stat fileBuffer; //buffer that holds file statistics

unsigned long fileLength = 0; // determines the length of the file

char buff = NULL;
/
********************************************/

int main()
{

/* This opens whatever named file appears under the FILENAME define. If that file does not exist, The program creates that file. */

//If fd == -1, then the file doesn't exist

if( fd = open( FILENAME, O_RDWR ) == -1 )
{
printf("Shared file does not exist: FD = %d\n", fd);

/* Since the file does not exist, we must create it, and supply the individual and group read write access rights to the file. */

fd = creat\( FILENAME, 0664 \);
printf\("creating %s :FD = %d\\n", FILENAME, fd \);

}//end if

/* If we've made it this far the file exist and it is opened */
printf\("File has been opened\\n"\);
stat \( FILENAME, &fileBuffer \);

while \( loopFlag == 0 \)
\{
  sleep\( 3 \);

  /* Enough time has passed for us to check the shared file
     to see if any requests have been written to it.  We need to
     lock the shared file before we attempt to open it, and
     unlock the file after we are done reading from it. */

  //The file needs to be locked from beginning to end.
  //lseek ensures that the file is locked from the beginning.

  lseek\( fd, 0, 0 \); /* Rewind file */

  if\( lockf\( fd, F_LOCK, 0 \) == 0 \)
  \{
    printf\("Shared file successfully locked.\\n"\);

    //The file has been successfully locked, now it must be read.


    /*The commented line below is what I want to use, but it
       locks up the program once read\(\) is called*/
    //fileLength = fileBuffer.st_size;
    
    buff = \(char *\) calloc\( MAXBUFF \+ 1, sizeof\( char \)\);
    printf\("FD: %d \\n", fd\); //debugging

    //ERROR OCCURS HERE if fileLength is anything other than 0

    bytesRead = read\( fd, buff, fileLength \);
    printf\("file has been read. Bytes read: %d\\n", bytesRead\);

  \}//end if

\}//end while

}//end main

The particular error that you are searching for is here:
if( fd = open( FILENAME, O_RDWR ) == -1 )
If the open system call succeeds, it will not return -1. Thus you will set fd=(open() == -1) to zero! You will not attempt the creat. Then you stat() the real file. Eventually you attempt a read from fd 0. You could just type something since you're reading from the terminal.

You gotta use:
if ((fd = open() ) == -1)
to get what you want. Don't feel too bad, this one still nails me.

You have several other bugs, but I'll let you find them for the most part. One bug is very subtle though. First you open() the file, if that fails then you creat() it. That is a race condition. Suppose the file is not there, so the open() fails. Just then, another process creates it. Then your creat() call must now fail too. Whenever you check to see if something is possible and then do it if looks possible, you are setting yourself up for this situation. It's ok to do a check first, but then you must check for success to see if it really worked.

Thanks so much my friend, for the life of me I couldn't figure out what it was, but your tip fixed my problem. Heh, I was dreaming in code last night, I was definitely purturbed that I couldn't figure what was going wrong. I will look at the rest of my code to figure out those bugs. Thanks alot for taking the time to help me, I owe you one :wink:

Peace.

-MJ

Excellent comment about the race condition!

One thing puzzles me, this code looks C++-ish?
Are // accepted as C comments, now?

Anyway, what I wanted to add, was that even though there is no _syntactical_ distinction between 'Procedure' and 'Function' in C, the _semantic_ one remains! So, unless a function returns void (the Function is not a "Function" but a "Procedure" :rolleyes: ) its return value must always be checked.
This is why Perderabo tells you that 'it's ok to check first, but you must check after'

I have a 'standard' way for dealing with opening/creation.

/*
Try to open, if that fails, _try_to_ create.
If create was successful, close.
Now, we have done what we can to make sure
we can open the file, so now try to open
and return the result.
*/
myopen(...)
{
if (fd = open(...) == -1) {
if (fd = creat(...) != -1)
close(fd);
fd = open(...);
}
return(fd);
}

It all depends on what you define to be an 'error'.

Yes, at least with gcc. No idea if that is a standard though.

What is the point of all of that? Why not just use open() with an O_CREAT flag and a mode argument?

Sorry, it was with reference to the original post, where the semantics was
open! Did it work? No?
Then create ... and continue ...

That, of course, lead to a bug.

(1)
So, I tried to illustrate a point that was no directly related to just opening and creating files, but to calling functions that might return errors in general.

(2)
Sometimes, the course of action may be different when the file is already there.

(3)
Also, I have gotten the habit of doing it that way from some old, long forgotten systems.

Atle