Board index » C Language

All times are UTC



 basic config file read 
Author Message
xex
#1 / 6
 basic config file read

Hi All -

I've written some very basic code to read a configuration file and
store the values.  Having been working mostly in Perl these days, C
string processing takes a little getting used to.  I'd just like to
know if I'm making any aggregious errors here.  Hopefully not!  The
code compiles and gives me the correct output, but I was a bit
concerned about memory alloc for the strings.  I'm going to continue
to enhance the code to make it more generic and useful, but before I
move on, I'd love to get some input from anyone who cares to share.

Thanks!
-Xexe

Here's the code:

#include <stdio.h>
#include <string.h>

#define LINE_SIZE 80
#define MAX_ITEM_LEN 20
#define MAX_CONFIG_ITEMS 100

struct cfg_item {
        char *name;
        char *value;

Quote:
} config[MAX_CONFIG_ITEMS];

int get_config();

int get_config(char *filename)
{
        /* assumes structure of "name\t=\tvalue" in the config file */
        char line[LINE_SIZE];
        char *name_ptr, *value_ptr;
        int i = 0;

        FILE *fp = fopen(filename, "r");
        if ( fp == 0 )  return -1;

        while (fgets(line, sizeof(line), fp)) {
                /* get rid of newline character */
                line[strlen(line) - 1] = '\0';

                name_ptr = line; /* the name of the config item is
first */

                value_ptr = strchr(line, '=') + 2; /* the value of the
config item is after the = and \t */

                name_ptr[4] = '\0'; /* should be changed to terminate
the string at the first sign of whitespace, */
                                    /* currently, the config items
have 4 char names */

                config[i].name = (char *)malloc(MAX_ITEM_LEN);
                config[i].name = strdup(name_ptr);
                config[i].value = (char *)malloc(MAX_ITEM_LEN);
                config[i].value = strdup(value_ptr);

                i++;
        }
        fclose(fp);
        return 0;

Quote:
}

main(int argc, char *argv[])
{
        int ok, i;

        ok = get_config(argv[1]); /* first arg is the config file name
*/

        for (i=0; i<2; i++) {
                printf("name:%s value:%s\n", config[i].name,
config[i].value);
        }

        return 0;

Quote:
}



Wed, 26 Nov 2003 07:44:42 GMT  
Russ Bobbi
#2 / 6
 basic config file read

Quote:
>Hi All -

>I've written some very basic code to read a configuration file and
>store the values.  Having been working mostly in Perl these days, C
>string processing takes a little getting used to.  I'd just like to
>know if I'm making any aggregious errors here.  Hopefully not!  The
>code compiles and gives me the correct output, but I was a bit
>concerned about memory alloc for the strings.  I'm going to continue
>to enhance the code to make it more generic and useful, but before I
>move on, I'd love to get some input from anyone who cares to share.

>Thanks!
>-Xexe

>Here's the code:

>#include <stdio.h>
>#include <string.h>

>#define LINE_SIZE 80
>#define MAX_ITEM_LEN 20
>#define MAX_CONFIG_ITEMS 100

>struct cfg_item {
>        char *name;
>        char *value;
>} config[MAX_CONFIG_ITEMS];

>int get_config();

int get_config(char *);

Your prototype doesn't match your definition below.

Quote:

>int get_config(char *filename)

Since you don't need to modify filename, this would be better as:

int get_config(const char *filename)

Quote:
>{
>        /* assumes structure of "name\t=\tvalue" in the config file */
>        char line[LINE_SIZE];
>        char *name_ptr, *value_ptr;
>        int i = 0;

>        FILE *fp = fopen(filename, "r");
>        if ( fp == 0 )  return -1;

>        while (fgets(line, sizeof(line), fp)) {
>                /* get rid of newline character */
>                line[strlen(line) - 1] = '\0';

It's possible that the line retrieved by fgets() is more than sizeof
line - 1 characters (unless you are comfortable with the assumption
that this can't happen). In this case, the newline will not appear at
the end of the string and your code above will delete the last
character.

I would insert a test, just in case:

char *tmp=strchr(line,'\n');

if (tmp!=NULL) {
    *tmp='\0';  /* all's ok, discard the newline */

Quote:
} else {

    /* line was truncated, either silently
     * ignore it, return an error code or
     * whatever's appropriate
     */

Quote:
}

>                name_ptr = line; /* the name of the config item is
>first */

>                value_ptr = strchr(line, '=') + 2; /* the value of the
>config item is after the = and \t */

I would be more cautious here, in case I've run into a "can't happen"
situation where the line doesn't contain an '='.  If there is no '=',
you will be assigning value_ptr the result of NULL + 2.  Or, consider
the possibility that the last character in line is an '=' in which
case you're possibly accessing memory you don't own.

Quote:

>                name_ptr[4] = '\0'; /* should be changed to terminate
>the string at the first sign of whitespace, */
>                                    /* currently, the config items
>have 4 char names */

 You can use strchr() to find out where the first '\t' occurs and then
set the value pointed to to '\0'.

char *tmp=strchr(name_ptr,'\t');

if (tmp!=NULL) {
    *tmp='\0';

Quote:
}

/* ... */

Quote:

>                config[i].name = (char *)malloc(MAX_ITEM_LEN);

No need to cast malloc()'s return value. See other threads and
http://www.eskimo.com/~scs/C-faq/q7.7.html.

Also, you might as well declare the name and value members of your
struct as arrays of char of size MAX_ITEM_LEN if you're going to take
this approach.  This would be more reasonable:

config[i].name=malloc(strlen(name_ptr)+1);  

Quote:
>                config[i].name = strdup(name_ptr);

strdup() isn't part of the standard C library so this reduces the
portability of your code.  Also, all implementations of strdup() that
I've seen allocate the necessary memory for you as well as copying the
string.  So, it would seem you're creating a memory leak. You're
allocating memory above and pointing config[i].name to it, and then
strdup() allocates more memory, copies the string and then points
config[i].name to it, causing you to lose your reference to the first
memory block.

Quote:
>                config[i].value = (char *)malloc(MAX_ITEM_LEN);
>                config[i].value = strdup(value_ptr);

See above.

Quote:

>                i++;

i won't retain its value across function invocations being that it's
an auto variable.  You can either declare it as static, declare it
outside of the function at file scope or keep track of it in your
main() function, modify this function to accept an index, while
incrementing the index in main() with each invocation of get_config().

Quote:
>        }
>        fclose(fp);
>        return 0;
>}

>main(int argc, char *argv[])
>{
>        int ok, i;

You should check to make sure you got at least 1 argument:

if (argc < 2) {
    fprintf(stderr,"usage: %s <file name>\n",argv[0]);
    return EXIT_FAILURE;  /* include <stdlib.h> above */

Quote:
}
>        ok = get_config(argv[1]); /* first arg is the config file name
>*/

>        for (i=0; i<2; i++) {
>                printf("name:%s value:%s\n", config[i].name,
>config[i].value);
>        }

>        return 0;
>}

HTH,

Russ



Wed, 26 Nov 2003 08:52:53 GMT  
CBFalcone
#3 / 6
 basic config file read

Quote:

> I've written some very basic code to read a configuration file and
> store the values.  Having been working mostly in Perl these days, C
> string processing takes a little getting used to.  I'd just like to
> know if I'm making any aggregious errors here.  Hopefully not!  The
> code compiles and gives me the correct output, but I was a bit
> concerned about memory alloc for the strings.  I'm going to continue
> to enhance the code to make it more generic and useful, but before I
> move on, I'd love to get some input from anyone who cares to share.

> Here's the code:

> #include <stdio.h>
> #include <string.h>

> #define LINE_SIZE 80
> #define MAX_ITEM_LEN 20
> #define MAX_CONFIG_ITEMS 100

> struct cfg_item {
>         char *name;
>         char *value;
> } config[MAX_CONFIG_ITEMS];

> int get_config();

You don't want this line - in fact it is harmful because there is
one more thing to maintain and that prototype is incomplete.  The
following line serves as a proper prototype by itself.

Quote:

> int get_config(char *filename)
> {
>         /* assumes structure of "name\t=\tvalue" in the config file */
>         char line[LINE_SIZE];
>         char *name_ptr, *value_ptr;
>         int i = 0;

>         FILE *fp = fopen(filename, "r");
>         if ( fp == 0 )  return -1;

>         while (fgets(line, sizeof(line), fp)) {
>                 /* get rid of newline character */
>                 line[strlen(line) - 1] = '\0';

Check there IS a newline char first.  If the input was not
completely read there won't be any, and there is more of the line
to be read.

Quote:

>                 name_ptr = line; /* the name of the config item is
> first */

>                 value_ptr = strchr(line, '=') + 2; /* the value of the
> config item is after the = and \t */

Check that you really found one.

Quote:

>                 name_ptr[4] = '\0'; /* should be changed to terminate
> the string at the first sign of whitespace, */

Why to keep line lengths short (under 72 at least) for publication
in newsgroups.

I see no name_ptr[4] above, just one lonely little char
*name_ptr.  You just blew up something or other.

Quote:
>                                     /* currently, the config items
> have 4 char names */

>                 config[i].name = (char *)malloc(MAX_ITEM_LEN);

Don't cast.  It hides errors you should be warned about.  Check
that it worked.

Quote:
>                 config[i].name = strdup(name_ptr);

You just destroyed the results of the malloc and created a memory
leak.  If you are going to do this you don't need the malloc at
all (but you still need to eventually destroy it).  You were
better off in the first place, because strdup is non-std and
non-portable.  You want strncpy() here.

Quote:
>                 config[i].value = (char *)malloc(MAX_ITEM_LEN);
>                 config[i].value = strdup(value_ptr);

Same comments as above.

Quote:

>                 i++;

What if this gets larger than the allocated space?  You need a
test somewhere.

Quote:
>         }
>         fclose(fp);
>         return 0;
> }

main returns an int.  Say so.

Quote:
> main(int argc, char *argv[])
> {
>         int ok, i;

>         ok = get_config(argv[1]); /* first arg is the config file name
> */

Is there any argv[1]?  You didn't test argc.  Ok is confusing
because getconfig returns zero for success, err would be a better
name.  You never tested it, so the following has no idea whether
it is working on valid data.  Nor did you return a count of input
items received, and are using magic numbers in the output loop.
Something like

                   items = getconfig(...);
                   if (items == 0) {
                      /* errormessage */
                   }
                   else if (items > 0) {
                      /* dump items things */
                   }

Quote:

>         for (i=0; i<2; i++) {
>                 printf("name:%s value:%s\n", config[i].name,
> config[i].value);
>         }

>         return 0;
> }

Use spaces, not tabs for publishing here.  Indentation levels of 3
or 4 are better.

Just a few mild criticisms.  Shows promise.

--

http://www.qwikpages.com/backstreets/cbfalconer :=(down for now)
   (Remove "NOSPAM." from reply address. my-deja works unmodified)



Wed, 26 Nov 2003 20:53:08 GMT  
xex
#4 / 6
 basic config file read
Thanks to both the posters who responded. I've incorporated your
suggestions and changed it to use strtok.  Seems to me like a more
robust version of the program, still some room to grow. :)

Thanks again,
Xexe

/*
  read_cfg.c - program to read config file
    args: name of config file
    return values: 0 for success, -1 for failure
    note: currently expects format of name\t=\tvalue in config file,
          ignores other formats
*/
#include <stdio.h>
#include <string.h>

#define LINE_SIZE 80
#define MAX_ITEM_LEN 20
#define MAX_CONFIG_ITEMS 100

struct cfg_item {
    char name[MAX_ITEM_LEN];
    char value[MAX_ITEM_LEN];

Quote:
} config[MAX_CONFIG_ITEMS];

int get_config(const char *filename)
{
    char line[LINE_SIZE];
    char *name, *value;
    char *t, *tmp, *delim;
    int i = 0;

    FILE *fp = fopen(filename, "r"); /* open configfile */
    if ( fp == 0 ) {
        fprintf(stderr, "Error opening %s\n", filename);
        return -1;
    }

    while (fgets(line, sizeof(line), fp)) {
        /* ignore comments, blank lines, lines beginning with space */
        if (line[0] == '#' || line[0] == ' ') continue;
        /* check for correct delimeter */
        delim = strstr(line, "\t=\t");
        if (delim == NULL) continue;
        /* get rid of newline at the end of the line */
        tmp = strchr(line,'\n');
        if (tmp != NULL) {
            line[strlen(line) - 1] = '\0';
        } else {
            fprintf(stderr, "Newline missing\n");
            return -1;
        }
        /* split line based on delimeter */
        name = strtok(line, "\t=\t");
        if (name == NULL) continue;
        strcpy(config[i].name, name);
        value = strtok(NULL, "\t=\t");
        if (value == NULL) continue;
        strcpy(config[i].value, value);
        i++;
        if (i > MAX_CONFIG_ITEMS) {
            fprintf(stderr, "Max num of config lines reached\n");
            return -1;
        }
    }
    fclose(fp);
    return i;

Quote:
}

int main(int argc, char *argv[])
{
    int config_items, i;

    if (argc < 2) {
        fprintf(stderr, "usage: %s <file name>\n", argv[0]);
        return -1;
    }

    config_items = get_config(argv[1]); /* 1st arg is config file name
*/

    if (config_items > 0) {
        printf("%d configuration items found\n", config_items);
        for (i = 0; i < config_items; i++) {
            printf("name:%s\tvalue:%s\n", config[i].name,
config[i].value);
        }
    } else {
        fprintf(stderr, "Errors detected, processing stopped\n");
        return -1;
    }

Quote:
}



Sat, 29 Nov 2003 06:14:49 GMT  
Stefan Farfeled
#5 / 6
 basic config file read

Quote:

>Thanks to both the posters who responded. I've incorporated your
>suggestions and changed it to use strtok.  Seems to me like a more
>robust version of the program, still some room to grow. :)

>Thanks again,
>Xexe

>/*
>  read_cfg.c - program to read config file
>    args: name of config file
>    return values: 0 for success, -1 for failure
>    note: currently expects format of name\t=\tvalue in config file,
>          ignores other formats
>*/
>#include <stdio.h>
>#include <string.h>

>#define LINE_SIZE 80
>#define MAX_ITEM_LEN 20
>#define MAX_CONFIG_ITEMS 100

>struct cfg_item {
>    char name[MAX_ITEM_LEN];
>    char value[MAX_ITEM_LEN];
>} config[MAX_CONFIG_ITEMS];

Have you considered avoiding this global array? You could pass it to
get_config, along with the number of its elements, thus gaining more
generality.

- Show quoted text -

Quote:
>int get_config(const char *filename)
>{
>    char line[LINE_SIZE];
>    char *name, *value;
>    char *t, *tmp, *delim;
>    int i = 0;

>    FILE *fp = fopen(filename, "r"); /* open configfile */
>    if ( fp == 0 ) {
>        fprintf(stderr, "Error opening %s\n", filename);
>        return -1;
>    }

>    while (fgets(line, sizeof(line), fp)) {
>        /* ignore comments, blank lines, lines beginning with space */
>        if (line[0] == '#' || line[0] == ' ') continue;
>        /* check for correct delimeter */
>        delim = strstr(line, "\t=\t");
>        if (delim == NULL) continue;
>        /* get rid of newline at the end of the line */
>        tmp = strchr(line,'\n');
>        if (tmp != NULL) {
>            line[strlen(line) - 1] = '\0';

Or the simpler *tmp = '\0';

Quote:
>        } else {
>            fprintf(stderr, "Newline missing\n");
>            return -1;

You should close the file before returning from get_config. Setting i to
-1 and using "break" will help.

Quote:
>        }
>        /* split line based on delimeter */
>        name = strtok(line, "\t=\t");

This will search for the first tab or '=' char, replace it with '\0' and
return line. Note that that two tabs aren't better than one. :-)
I think *delim = '\0' is what you want.

Quote:
>        if (name == NULL) continue;
>        strcpy(config[i].name, name);

To be really robust, you should check if name fits into your array
config[i].name. You could either use dynamical allocation to allow
arbitrary lengths of the name, issue an error if it is too long or
simply accept only the first MAX_ITEM_LEN - 1 chars.

Quote:
>        value = strtok(NULL, "\t=\t");

Like above. The pointer delim + 3 points after the "\t=\t".

- Show quoted text -

Quote:
>        if (value == NULL) continue;
>        strcpy(config[i].value, value);
>        i++;
>        if (i > MAX_CONFIG_ITEMS) {
>            fprintf(stderr, "Max num of config lines reached\n");
>            return -1;
>        }
>    }
>    fclose(fp);
>    return i;
>}

>int main(int argc, char *argv[])
>{
>    int config_items, i;

>    if (argc < 2) {
>        fprintf(stderr, "usage: %s <file name>\n", argv[0]);
>        return -1;

ISO C only defines semantics for the values 0, EXIT_SUCCESS and
EXIT_FAILURE.

Quote:
>    }

>    config_items = get_config(argv[1]); /* 1st arg is config file name
>*/

>    if (config_items > 0) {
>        printf("%d configuration items found\n", config_items);
>        for (i = 0; i < config_items; i++) {
>            printf("name:%s\tvalue:%s\n", config[i].name,
>config[i].value);
>        }
>    } else {
>        fprintf(stderr, "Errors detected, processing stopped\n");
>        return -1;

See above.

Quote:
>    }

return 0;

Quote:
>}

--
Stefan Farfeleder
C FAQ at <http://www.eskimo.com/~scs/C-faq/top.html>


Sat, 29 Nov 2003 06:51:22 GMT  
Barry Schwar
#6 / 6
 basic config file read
See annotations in code.


Quote:
>Thanks to both the posters who responded. I've incorporated your
>suggestions and changed it to use strtok.  Seems to me like a more
>robust version of the program, still some room to grow. :)

>Thanks again,
>Xexe

>/*
>  read_cfg.c - program to read config file
>    args: name of config file
>    return values: 0 for success, -1 for failure
>    note: currently expects format of name\t=\tvalue in config file,
>          ignores other formats
>*/
>#include <stdio.h>
>#include <string.h>

>#define LINE_SIZE 80
>#define MAX_ITEM_LEN 20
>#define MAX_CONFIG_ITEMS 100

>struct cfg_item {
>    char name[MAX_ITEM_LEN];
>    char value[MAX_ITEM_LEN];
>} config[MAX_CONFIG_ITEMS];

>int get_config(const char *filename)
>{
>    char line[LINE_SIZE];
>    char *name, *value;
>    char *t, *tmp, *delim;
>    int i = 0;

>    FILE *fp = fopen(filename, "r"); /* open configfile */
>    if ( fp == 0 ) {
>        fprintf(stderr, "Error opening %s\n", filename);
>        return -1;
>    }

>    while (fgets(line, sizeof(line), fp)) {
>        /* ignore comments, blank lines, lines beginning with space */
>        if (line[0] == '#' || line[0] == ' ') continue;
>        /* check for correct delimeter */
>        delim = strstr(line, "\t=\t");
>        if (delim == NULL) continue;
>        /* get rid of newline at the end of the line */
>        tmp = strchr(line,'\n');
>        if (tmp != NULL) {
>            line[strlen(line) - 1] = '\0';

If the only place you care about a '\n' is the last character before
the '\0', then why not test that character.  Why expend the overhead
of strchr().  If you are going to expend that overhead, take advantage
of the result and use *tmp = '\0';
Quote:
>        } else {
>            fprintf(stderr, "Newline missing\n");
>            return -1;

You already use -1 to indicate an open failure.  Why not use a
different negative to indicate bad data in the file.

Why is a missing '\n' a fatal error but a missing "\t=\t" not?

Quote:
>        }
>        /* split line based on delimeter */
>        name = strtok(line, "\t=\t");

This does not find the "\t=\t" triplet.  In finds the first occurrence
of '\t' or '=', sets that character to '\0', and returns a pointer to
line.   If there are any '\t' or '=' characters that are before your
intended triplet, it will find them instead.  The second '\t' is
useless and misleading.  If it finds the '\t' that starts your
intended triplet, it will replace it with a '\0'.
Quote:
>        if (name == NULL) continue;

This can never be true because you already have a non-NULL delim from
the call to strstr or you would not reach this point.
Quote:
>        strcpy(config[i].name, name);

Are you sure it will fit?
Quote:
>        value = strtok(NULL, "\t=\t");
>        if (value == NULL) continue;

This if statement can never evaluate to true.
Quote:
>        strcpy(config[i].value, value);

Are you sure it will fit?
Quote:
>        i++;
>        if (i > MAX_CONFIG_ITEMS) {
>            fprintf(stderr, "Max num of config lines reached\n");

This error is premature.  You have "successfully" stored the line just
read and you do not yet know if there is another line that would
overflow.
Quote:
>            return -1;

-1 again?

You have an open file that needs to be closed.

Quote:
>        }
>    }
>    fclose(fp);
>    return i;
>}

I think what you want to do is keep delim pointing at the triplet and
finish checking the card format.  Then set *delim to '\0', copy
config[i].name from line, and copy config[i].value from delim+3

- Show quoted text -

Quote:

>int main(int argc, char *argv[])
>{
>    int config_items, i;

>    if (argc < 2) {
>        fprintf(stderr, "usage: %s <file name>\n", argv[0]);
>        return -1;
Use EXIT_FAILURE
>    }

>    config_items = get_config(argv[1]); /* 1st arg is config file name
>*/

>    if (config_items > 0) {
>        printf("%d configuration items found\n", config_items);
>        for (i = 0; i < config_items; i++) {
>            printf("name:%s\tvalue:%s\n", config[i].name,
>config[i].value);
>        }
>    } else {
>        fprintf(stderr, "Errors detected, processing stopped\n");
>        return -1;
EXIT_FAILURE
>    }

return EXIT_SUCCESS;

Quote:
>}

<<Remove the del for email>>


Sat, 29 Nov 2003 12:26:33 GMT  
 
 Page 1 of 1
  [ 6 post ] 

 Relevant Pages 

1. Config files: best to use struct when reading in data

2. read Config-file module for C?

3. reading config files

4. are there better ways to read config file?

5. How to read a config file in C

6. looking for function to read config file

7. Library for reading config files?

8. read config file

9. Updating the security.config file in the CONFIG directory

10. Help!, Can this file structure be read in visual basic

11. Can't read app.config....

12. cannot read web.config

  

 
Powered by phpBB® Forum Software