fgets problem

Discussion in 'C Programming' started by Eric Sosman, Dec 22, 2008.

  1. Eric Sosman

    Eric Sosman Guest

    Alan Peake wrote:
    > Hi all,
    > I've been trying to read a text file into an array without much
    > success. The following is part of "main" and all appears OK until the
    > statement while (fgets(str,20,fp) !=NULL)
    > Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    > perfectly and the program performs as expected. But more often than not,
    > it appears to treat the first line of xtl.dat (which is "0.51400) as
    > NULL and skips the rest of the file.
    > Can anyone see what I've done wrong?
    > Thanks
    > Alan
    >
    > #include "stdio.h"


    Should be <stdio.h>, with angle brackets.

    > #include <math.h>
    > #include <stdlib.h>
    >
    > float f[140] ;
    >
    > main(argc,argv)
    > int argc ;
    > char *argv[] ;


    Not the source of your problem, but this is very old-
    fashioned, almost two decades out of date. Replace all
    three lines with

    int main(int argc, char *argv[])

    .... to use modern style (where "modern" means "more recent
    than the Reagan administration").

    > {
    > FILE *fopen() , *fp ;
    > float freq=20.0,tol=0.0005 ;
    > float freqin(),tolin() ;
    > char *str,c ;


    Here's the beginning of your problem, although the problem
    doesn't occur quite yet. `str' is a variable that can point
    to a char, and you plan to use it to point to the first of a
    bunch of chars. But what does it point at NOW? You haven't
    given it a value, so its value is indeterminate. Informally,
    it "points to nowhere," which is a perfectly acceptable state
    of affairs at the moment, but ...

    > int i=0,j,k,size=120 ;
    > if ((fp=fopen(argv[1],"r")) == NULL) {
    > printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    > }
    > while (fgets(str,20,fp) != NULL) {


    .... but now fgets() will try to store as many as 20 characters
    at and after the place `str' points to. Where does `str' point?
    See above, and see Question 7.2 in the comp.lang.c Frequently
    Asked Questions (FAQ) list at <http://www.c-faq.com/>.

    > f=atof(str) ;


    Also not the source of your problem, but strtod() is more
    robust than atof(), because it lets you discover the error
    when the input looks like "1.2.3" or "x44".

    > for (i=0;i<130;i++) {
    > fgets(str,20,fp) ;


    Same problem as above: where does `str' point?

    > f = atof(str) ;


    Note that the first time through the loop you will wipe
    out the value that was previously stored in f[0]. Is that
    what you meant to do? If so, why did you bother storing
    the earlier f[0] value?

    > printf(" %10s %3.6f %d",str,f,i) ;
    > }
    > fclose(fp) ;
    >


    --
    Eric Sosman
    lid
     
    Eric Sosman, Dec 22, 2008
    #1
    1. Advertising

  2. Alan Peake <> writes:
    > I've been trying to read a text file into an array without much
    > success. The following is part of "main" and all appears OK until the
    > statement while (fgets(str,20,fp) !=NULL)
    > Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    > perfectly and the program performs as expected. But more often than
    > not, it appears to treat the first line of xtl.dat (which is "0.51400)
    > as NULL and skips the rest of the file.
    > Can anyone see what I've done wrong?
    > Thanks
    > Alan
    >
    > #include "stdio.h"
    > #include <math.h>
    > #include <stdlib.h>
    >
    > float f[140] ;
    >
    > main(argc,argv)
    > int argc ;
    > char *argv[] ;
    > {
    > FILE *fopen() , *fp ;
    > float freq=20.0,tol=0.0005 ;
    > float freqin(),tolin() ;
    > char *str,c ;
    > int i=0,j,k,size=120 ;
    > if ((fp=fopen(argv[1],"r")) == NULL) {
    > printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    > }
    > while (fgets(str,20,fp) != NULL) {
    > f=atof(str) ;
    > for (i=0;i<130;i++) {
    > fgets(str,20,fp) ;
    > f = atof(str) ;
    > printf(" %10s %3.6f %d",str,f,i) ;
    > }
    > fclose(fp) ;


    The code you posted is incomplete; you're missing closing braces for
    the while loop and for the main function. Judging by the indentation,
    you call fopen() exactly once, but you call fclose() on each iteration
    of the while loop. Calling fgets() on a closed file invokes undefined
    behavior. I suspect that's the cause of your problem, but it's hard
    to be sure without seeing your input.

    There are a number of other things you should fix; some of them are
    errors, some are style issues.

    Include directives for standard headers should use <>, not "".
    Change
    #include "stdio.h"
    to
    #include <stdio.h>

    You don't use anything from <math.h>, so that header isn't necessary.
    This might be an artifact of the way you've trimmed the program for
    posting (thank you for that).

    Your array "f" is used only inside main, so there's no reason to
    declare it at file scope. Again, this might be an artifact of the way
    you've trimmed the program for posting, but it might still make more
    sense to declare it in one function and pass a pointer to other
    functions that use it.

    I personally dislike putting a space in front of each semicolon, and I
    always put a space after each comma. Not all C programmers share this
    particular opinion, but I think most do.

    Your declaration of main is an old-style declaration; there's no
    longer any good reason to use such declarations. Use:
    int main(int argc, char *argv[])
    (I personally prefer "char **argv", but "char *argv[]" is certainly
    valid.)

    There's no need to re-declare fopen, and in fact it's a bad idea to do
    so; <stdio.h> does that for you.

    I find code that declares one thing per line to be clearer:
    float freq = 20.0;
    float tol = 0.0005;

    It usually makes more sense to use double rather than float; it
    usually provides more precision and range, and on many CPUs it's just
    as fast.

    You declare freqin() and tolin(), but you don't define or use them.
    If they exist in your actual code, it would be better to provide
    prototypes at file scope, probably in a header file. It's legal to
    have function declarations inside a function definition, but it's
    generally a bad idea.

    exit(1) is, strictly speaking, not portable; use exit(EXIT_FAILURE)
    unless you're sure that you specifically want to pass a status of 1 to
    the calling environment.

    130 is a "magic number"; there's nothing in your program that explains
    why it has that value. Declare a constant, perhaps using #define.

    In the loop, you call fgets() without checking the result. This is a
    bad idea; you have no way of detecting failure. This makes your
    program "brittle" in the presence of bad input data. You should also
    think about what to do if an input line is longer than what your array
    can hold.

    Don't use the atof() function; it doesn't detect errors, and in fact
    can invoke undefined behavior in some cases. (The exact circumstances
    in which the behavior is undefined are not entirely clear, which is
    another good reason not to use it.) Use strtod() instead; it's a
    little more complicated, but the extra safety is well worth i.

    All your output, other than the first line, is on a single line with
    no '\n' characters. Was that your intent? Strictly speaking, if your
    implementation requires a terminating '\n' at the end of the last line
    of output, then failing to provide one can invoke undefined behavior
    (though on most systems you'll just get a long line with no
    end-of-line marker at the end) Perhaps you wanted:
    printf(" %10s %3.6f %d\n", str, f, i);

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 22, 2008
    #2
    1. Advertising

  3. Eric Sosman <> writes:
    > Alan Peake wrote:

    [...]
    >> {
    >> FILE *fopen() , *fp ;
    >> float freq=20.0,tol=0.0005 ;
    >> float freqin(),tolin() ;
    >> char *str,c ;

    >
    > Here's the beginning of your problem, although the problem
    > doesn't occur quite yet. `str' is a variable that can point
    > to a char, and you plan to use it to point to the first of a
    > bunch of chars. But what does it point at NOW? You haven't
    > given it a value, so its value is indeterminate. Informally,
    > it "points to nowhere," which is a perfectly acceptable state
    > of affairs at the moment, but ...
    >
    >> int i=0,j,k,size=120 ;
    >> if ((fp=fopen(argv[1],"r")) == NULL) {
    >> printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    >> }
    >> while (fgets(str,20,fp) != NULL) {

    >
    > ... but now fgets() will try to store as many as 20 characters
    > at and after the place `str' points to. Where does `str' point?

    [...]

    Argh, how did I miss that?

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 22, 2008
    #3
  4. Eric Sosman

    Eric Sosman Guest

    Keith Thompson wrote:
    > Alan Peake <> writes:
    >> [...]

    > [...]
    > you call fopen() exactly once, but you call fclose() on each iteration
    > of the while loop. [...]


    Argh, how did I miss that?

    --
    Eric Sosman
    lid
     
    Eric Sosman, Dec 22, 2008
    #4
  5. Eric Sosman

    CBFalconer Guest

    Alan Peake wrote:
    >
    > I've been trying to read a text file into an array without much
    > success. The following is part of "main" and all appears OK until
    > the statement while (fgets(str,20,fp) !=NULL)
    >
    > Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    > perfectly and the program performs as expected. But more often
    > than not, it appears to treat the first line of xtl.dat (which is
    > "0.51400) as NULL and skips the rest of the file.
    > Can anyone see what I've done wrong?


    See the description for cases where a NULL is returned:

    7.19.7.2 The fgets function

    Synopsis
    [#1]
    #include <stdio.h>
    char *fgets(char * restrict s, int n,
    FILE * restrict stream);

    Description

    [#2] The fgets function reads at most one less than the
    number of characters specified by n from the stream pointed
    to by stream into the array pointed to by s. No additional
    characters are read after a new-line character (which is
    retained) or after end-of-file. A null character is written
    immediately after the last character read into the array.

    Returns

    [#3] The fgets function returns s if successful. If end-of-
    file is encountered and no characters have been read into
    the array, the contents of the array remain unchanged and a
    null pointer is returned. If a read error occurs during the
    operation, the array contents are indeterminate and a null
    pointer is returned.

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Dec 23, 2008
    #5
  6. Eric Sosman

    Guest

    On Dec 23, 2:26 am, CBFalconer <> wrote:
    > Alan Peake wrote:
    >
    > > I've been trying to read a text file into an array without much
    > > success. The following is part of "main" and all appears OK until
    > > the statement while (fgets(str,20,fp) !=NULL)

    >
    > > Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    > > perfectly and the program performs as expected. But more often
    > > than not, it appears to treat the first line of xtl.dat (which is
    > > "0.51400) as NULL and skips the rest of the file.
    > > Can anyone see what I've done wrong?

    >
    > See the description for cases where a NULL is returned:


    The description is useless and the results meaningless if before fgets
    returns from it's caller undefined behavior is invoked. Before
    instructing someone to look at the description you should make sure
    that this doesn't happend. You didn't make sure. It did happend. The
    description is irrelevant.

    I'm curious, why do you choose to read only parts of a post? I guess
    this question will go unanswered since you probably stopped reading
    some lines ago.
     
    , Dec 23, 2008
    #6
  7. Eric Sosman

    CBFalconer Guest

    wrote:
    > CBFalconer <> wrote:
    >> Alan Peake wrote:
    >>
    >>> I've been trying to read a text file into an array without much
    >>> success. The following is part of "main" and all appears OK until
    >>> the statement while (fgets(str,20,fp) !=NULL)
    >>>
    >>> Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    >>> perfectly and the program performs as expected. But more often
    >>> than not, it appears to treat the first line of xtl.dat (which is
    >>> "0.51400) as NULL and skips the rest of the file.
    >>> Can anyone see what I've done wrong?

    >>
    >> See the description for cases where a NULL is returned:

    >
    > The description is useless and the results meaningless if before
    > fgets returns from it's caller undefined behavior is invoked.
    > Before instructing someone to look at the description you should
    > make sure that this doesn't happend. You didn't make sure. It did
    > happend. The description is irrelevant.


    You seem to feel that an incomplete and uncompilable program
    listing is adequate data. I disagree.

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Dec 23, 2008
    #7
  8. On Tue, 23 Dec 2008 08:37:59 +0000, Alan Peake
    <> wrote:

    >Hi all,
    > I've been trying to read a text file into an array without much
    >success. The following is part of "main" and all appears OK until the
    >statement while (fgets(str,20,fp) !=NULL)
    >Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    >perfectly and the program performs as expected. But more often than not,
    >it appears to treat the first line of xtl.dat (which is "0.51400) as
    >NULL and skips the rest of the file.
    >Can anyone see what I've done wrong?
    >Thanks
    >Alan
    >
    > #include "stdio.h"
    > #include <math.h>
    > #include <stdlib.h>
    >
    > float f[140] ;
    >
    > main(argc,argv)
    > int argc ;
    > char *argv[] ;
    > {
    > FILE *fopen() , *fp ;


    You include stdio.h. There is no reason to attempt to declare it here
    also, especially since you do so incorrectly.

    > float freq=20.0,tol=0.0005 ;
    > float freqin(),tolin() ;


    You don't call these functions so this line seems pointless.

    > char *str,c ;
    > int i=0,j,k,size=120 ;
    > if ((fp=fopen(argv[1],"r")) == NULL) {
    > printf("No crystal list file %s\n",argv[1]) ; exit(1) ;


    1 is not a portable return value from main. Use EXIT_FAILURE which is
    defined in stdlib.h which you already include.

    > }
    > while (fgets(str,20,fp) != NULL) {
    > f=atof(str) ;


    atof does not provide any indication that it was successful. Use
    strtod instead.

    > for (i=0;i<130;i++) {
    > fgets(str,20,fp) ;
    > f = atof(str) ;


    In the first iteration of this loop, you replace the value stored in
    f[0] above.

    > printf(" %10s %3.6f %d",str,f,i) ;
    > }
    > fclose(fp) ;


    --
    Remove del for email
     
    Barry Schwarz, Dec 23, 2008
    #8
  9. Eric Sosman

    Guest

    On Dec 23, 3:44 pm, Alan Peake <>
    wrote:
    > Keith Thompson wrote:
    >
    > > The code you posted is incomplete; you're missing closing braces for
    > > the while loop and for the main function.

    >
    > I left out the rest of main for brevity. I've included most of yours and
    > other suggestions and corrections.
    >
    >
    >
    > > 130 is a "magic number"; there's nothing in your program that explains
    > > why it has that value. Declare a constant, perhaps using #define.

    >
    > The file that is used in argv[] has 130 text lines and I didn't know how
    > to set f[] to some number in the case of a different input file with
    > more or less entries.
    > Bit rusty on things like alloc :)


    Troll.
     
    , Dec 23, 2008
    #9
  10. writes:
    > On Dec 23, 3:44 pm, Alan Peake <>
    > wrote:
    >> Keith Thompson wrote:
    >>
    >> > The code you posted is incomplete; you're missing closing braces for
    >> > the while loop and for the main function.

    >>
    >> I left out the rest of main for brevity. I've included most of yours and
    >> other suggestions and corrections.
    >>
    >> > 130 is a "magic number"; there's nothing in your program that explains
    >> > why it has that value. Declare a constant, perhaps using #define.

    >>
    >> The file that is used in argv[] has 130 text lines and I didn't know how
    >> to set f[] to some number in the case of a different input file with
    >> more or less entries.
    >> Bit rusty on things like alloc :)

    >
    > Troll.


    Why on Earth would you assume that? He said his C is a bit rusty;
    what basis do you have for accusing him of dishonesty?

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 23, 2008
    #10
  11. Alan Peake <> writes:
    > Keith Thompson wrote:
    >> The code you posted is incomplete; you're missing closing braces for
    >> the while loop and for the main function.

    > I left out the rest of main for brevity. I've included most of yours
    > and other suggestions and corrections.


    Brevity is good (the soul of wit and all that), but posting complete
    compilable code is (almost) always a good idea. You were just two
    closing braces short of that.

    >> 130 is a "magic number"; there's nothing in your program that
    >> explains
    >> why it has that value. Declare a constant, perhaps using #define.

    >
    >
    > The file that is used in argv[] has 130 text lines and I didn't know how
    > to set f[] to some number in the case of a different input file with
    > more or less entries.
    > Bit rusty on things like alloc :)


    Your f array has 140 elements, and your code assumes 130 lines of
    input text. I had missed your assumption that these two values were
    related, and I'm guessing they were meant to be equal. That's an even
    better reason to use a named constant; you can change the value of the
    constant without having to change every reference to it. For example:

    #define EXPECTED_LINES 140
    ...
    float f[EXPECTED_LINES};
    ...
    for (i = 0; i < EXPECTED_LINES; i ++)
    ...

    If you want to use a fixed number of lines (which makes for simpler
    coding), you can print an error message if you detect that the file as
    more or fewer lines. If you want to allow for an arbitrary number of
    lines, and you need to store information from all of them, there are
    several solutions. You can declare a huge array and use only part of
    it, though that's wasteful. You can expand the array as needed using
    realloc(). You can use a linked list rather than an array.

    Incidentally, this newsgroup has a very good FAQ at
    <http://www.c-faq.com/>.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Dec 23, 2008
    #11
  12. Eric Sosman

    Guest

    On Dec 23, 7:31 am, Keith Thompson <> wrote:
    > writes:
    > > On Dec 23, 3:44 pm, Alan Peake <>
    > > wrote:
    > >> Keith Thompson wrote:

    >
    > >> > The code you posted is incomplete; you're missing closing braces for
    > >> > the while loop and for the main function.

    >
    > >> I left out the rest of main for brevity. I've included most of yours and
    > >> other suggestions and corrections.

    >
    > >> > 130 is a "magic number"; there's nothing in your program that explains
    > >> > why it has that value. Declare a constant, perhaps using #define.

    >
    > >> The file that is used in argv[] has 130 text lines and I didn't know how
    > >> to set f[] to some number in the case of a different input file with
    > >> more or less entries.
    > >> Bit rusty on things like alloc :)

    >
    > > Troll.

    >
    > Why on Earth would you assume that? He said his C is a bit rusty;
    > what basis do you have for accusing him of dishonesty?


    Maybe I'm just paranoid, but the last sentence gave him away I think.
    He also doesn't have a real e-mail address. The formatting is almost
    excellent (in the first post) except the semicolons.
    As for including "stdio.h" instead <stdio.h>, how can you miss
    something like that when the other two headers use <>? He top-posted.
    He also did

    char *str, c;

    Then

    toupper(c = getchar())

    I don't think that's just coincidental. How possible could it be that
    someone whose C is "rusty" to remember char *str, c; actually means
    char *str; char c; and what are the chances for him to use that object
    to save the getchar value returned, and then pass that to toupper?
    That's so subtly wrong and right (if c is changed to int) that I
    honestly think he's trying to get you all to talk about getchar &
    toupper. He's another Cunningham for christ's sake!

    One thing that hasn't been mentioned so far is that he's using argv[1]
    in fopen but he doesn't really check if argc > 1.
    He also decided to add some cls and khbit calls. (non-standard)
    He snipped code for "brevity"

    I just don't know how you can get so many things wrong on 3 posts (and
    I only mentioned _some_ of the things he got wrong). I imagine that
    he'll probably continue asking such questions and posting totally
    wrong code and you'll all continue wasting your time with him. Well,
    have fun, but I think I'll pass.
     
    , Dec 23, 2008
    #12
  13. Eric Sosman

    Alan Peake Guest

    Hi all,
    I've been trying to read a text file into an array without much
    success. The following is part of "main" and all appears OK until the
    statement while (fgets(str,20,fp) !=NULL)
    Sometimes it works fine and the file xtl.dat (argv[1]) is read in
    perfectly and the program performs as expected. But more often than not,
    it appears to treat the first line of xtl.dat (which is "0.51400) as
    NULL and skips the rest of the file.
    Can anyone see what I've done wrong?
    Thanks
    Alan

    #include "stdio.h"
    #include <math.h>
    #include <stdlib.h>

    float f[140] ;

    main(argc,argv)
    int argc ;
    char *argv[] ;
    {
    FILE *fopen() , *fp ;
    float freq=20.0,tol=0.0005 ;
    float freqin(),tolin() ;
    char *str,c ;
    int i=0,j,k,size=120 ;
    if ((fp=fopen(argv[1],"r")) == NULL) {
    printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    }
    while (fgets(str,20,fp) != NULL) {
    f=atof(str) ;
    for (i=0;i<130;i++) {
    fgets(str,20,fp) ;
    f = atof(str) ;
    printf(" %10s %3.6f %d",str,f,i) ;
    }
    fclose(fp) ;
     
    Alan Peake, Dec 23, 2008
    #13
  14. Eric Sosman

    Phil Carmody Guest

    Alan Peake <> writes:
    > float f[140] ;
    >
    > int main(int argc,char *argv[])
    > {
    > FILE *fp ;
    > float freq=20.0,tol=0.0005 ;
    > float freqin(),tolin() ;
    > char *str,c ;
    > int i=0,size ;
    > if ((fp=fopen(argv[1],"r")) == NULL) {
    > printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    > }
    > str=f ;
    > while (fgets(str,20,fp) != NULL) {


    So you're writing those chars into your array of floats.

    > f = atof(str) ;


    And then you're putting floats into your array of floats.

    > i++ ;
    > printf(" %10s %3.6f %d",str,f,i) ;
    > }


    And you're looping back to write more chars into your array of floats.

    > size =i ;


    You didn't expect the floats you wrote into that array of floats
    to still be in there, did you?

    If you want an array of chars into which to write, just define
    an array of chars and use that. Don't try and use the same
    block of memory for two different purposes at the same time;
    that way all kind of unwanted troubles lie.

    So scrap "char*str;" and "str=f", and use "char str[20];" instead.
    For improved sanity, don't use the magic number '20' more than
    once. Either use a sensibly named #define multiple times, or use
    sizeof to retrieve the array's size.

    Phil
    --
    I tried the Vista speech recognition by running the tutorial. I was
    amazed, it was awesome, recognised every word I said. Then I said the
    wrong word ... and it typed the right one. It was actually just
    detecting a sound and printing the expected word! -- pbhj on /.
     
    Phil Carmody, Dec 23, 2008
    #14
  15. Eric Sosman

    Alan Peake Guest

    Many thanks Eric, that seems to be the problem. Not sure of the proper
    fix but I did a "shonky" and let str=f. The compiler gave a warning
    about different types of course but the program now works reliably
    except that the printf(" %10s bit doesn't work. So I need to find out
    how to point str a bit better :)
    I've included your suggestions re int main but left atof until I can
    work out how strtod works. Anyway, I've put all of main here for any
    further comment.
    Some of the previous code was a bit messy as I was trying to find the
    error using CodeView. I also haven't used C 5.0 for about 10 years and
    the since retirement, the old brain doesn't work quite so well :)
    Cheers and thanks again,
    Alan



    #include <stdio.h>
    #include <math.h>
    #include <stdlib.h>

    float f[140] ;

    int main(int argc,char *argv[])
    {
    FILE *fp ;
    float freq=20.0,tol=0.0005 ;
    float freqin(),tolin() ;
    char *str,c ;
    int i=0,size ;
    if ((fp=fopen(argv[1],"r")) == NULL) {
    printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    }
    str=f ;
    while (fgets(str,20,fp) != NULL) {
    f = atof(str) ;
    i++ ;
    printf(" %10s %3.6f %d",str,f,i) ;
    }
    size =i ;
    while (!kbhit()) ;
    fclose(fp) ;
    cls() ;
    do {
    dca(1,1) ; printf("0 exit ") ;
    dca(2,1) ; printf("1 Enter desired frequency in MHz") ;
    dca(3,1) ; printf("2 Enter tolerance in MHz ") ;
    dca(4,1) ; printf("3 Find crystals ") ;
    dca(5,1) ; printf("4 Try all combinations ") ;
    switch (toupper(c=getchar())) {
    case '1' : freq = freqin() ; break ;
    case '2' : tol = tolin() ; break ;
    case '3' : findxt(freq,tol,size) ; break ;
    case '4' : tryall(freq,tol,size) ; break ;
    default : break ;
    }
    } while (c!='0') ;
    exit(0) ;
    }


    Eric Sosman wrote:

    >
    > Here's the beginning of your problem, although the problem
    > doesn't occur quite yet. `str' is a variable that can point
    > to a char, and you plan to use it to point to the first of a
    > bunch of chars. But what does it point at NOW? You haven't
    > given it a value, so its value is indeterminate. Informally,
    > it "points to nowhere," which is a perfectly acceptable state
    > of affairs at the moment, but ...
    >
    >> int i=0,j,k,size=120 ;
    >> if ((fp=fopen(argv[1],"r")) == NULL) {
    >> printf("No crystal list file %s\n",argv[1]) ; exit(1) ;
    >> }
    >> while (fgets(str,20,fp) != NULL) {

    >
    >
    > ... but now fgets() will try to store as many as 20 characters
    > at and after the place `str' points to. Where does `str' point?
     
    Alan Peake, Dec 23, 2008
    #15
  16. In article <>,
    Keith Thompson <> wrote:
    ....
    >> Troll.

    >
    >Why on Earth would you assume that? He said his C is a bit rusty;
    >what basis do you have for accusing him of honesty?


    Fixed it for you. No charge.

    The fact is that, in this newsgroup, the "trolls" are the only ones that
    speak truth.
     
    Kenny McCormack, Dec 23, 2008
    #16
  17. Eric Sosman

    Alan Peake Guest

    Keith Thompson wrote:

    >
    > The code you posted is incomplete; you're missing closing braces for
    > the while loop and for the main function.

    I left out the rest of main for brevity. I've included most of yours and
    other suggestions and corrections.


    >
    > 130 is a "magic number"; there's nothing in your program that explains
    > why it has that value. Declare a constant, perhaps using #define.



    The file that is used in argv[] has 130 text lines and I didn't know how
    to set f[] to some number in the case of a different input file with
    more or less entries.
    Bit rusty on things like alloc :)

    Alan
     
    Alan Peake, Dec 23, 2008
    #17
  18. On 23 Dec 2008 at 7:14, wrote:
    > Maybe I'm just paranoid


    Say, d'ya reckon?

    > I just don't know how you can get so many things wrong on 3 posts


    Try asking CBF - he's a past master at getting things wrong in post
    after post.
     
    Antoninus Twink, Dec 23, 2008
    #18
  19. Alan Peake <> writes:

    A few more remarks...

    > #include <stdio.h>
    > #include <math.h>
    > #include <stdlib.h>
    >
    > float f[140] ;


    I'd name the size for later use:

    #define N_FLOATS 140

    float f[N_FLOATS];

    and I'd use another name if I knew what these numbers were (float
    angles[N_ANGLES]; for example).

    > int main(int argc,char *argv[])
    > {
    > FILE *fp ;
    > float freq=20.0,tol=0.0005 ;
    > float freqin(),tolin() ;
    > char *str,c ;
    > int i=0,size ;
    > if ((fp=fopen(argv[1],"r")) == NULL) {


    You might as well add:

    if (argc != 2 || (fp = fopen(argv[1], "r")) == NULL) {

    That way you don't try to open a file when no name is given. You then
    need to fix the message:

    > printf("No crystal list file %s\n",argv[1]) ; exit(1) ;


    Which I'd do like this:

    printf("No crystal list file %s\n", argc > 1 ? argv[1] : "name provided");

    but there are lots of other less sneaky solutions.

    > }
    > str=f ;
    > while (fgets(str,20,fp) != NULL) {


    You need a separate array (as already mentioned). After

    char str[20];

    you can write fgets(str, sizeof str, fp), but for simple input like
    this I actually prefer the much maligned fscanf. I also prefer to
    check that I can put the data somewhere, even if I don't report the
    error of running out of room in a first draft of the program:

    while (i < N_FLOATS && fscanf("%f", &f) == 1) {

    It is a good habit to get into.

    <snip>
    It is now hours after I started the post so I am sure other people
    have said similar things, but I'd already typed too much to abandon
    posting!

    --
    Ben.
     
    Ben Bacarisse, Dec 23, 2008
    #19
  20. Eric Sosman

    CBFalconer Guest

    Alan Peake wrote:
    >
    > Many thanks Eric, that seems to be the problem. Not sure of the
    > proper fix but I did a "shonky" and let str=f. The compiler gave
    > a warning about different types of course but the program now
    > works reliably except that the printf(" %10s bit doesn't work.
    > So I need to find out how to point str a bit better :)


    Please do not top-post. Your answer belongs after (or intermixed
    with) the quoted material to which you reply, after snipping all
    irrelevant material. See the following links:

    <http://www.catb.org/~esr/faqs/smart-questions.html>
    <http://www.caliburn.nl/topposting.html>
    <http://www.netmeister.org/news/learn2quote.html>
    <http://cfaj.freeshell.org/google/> (taming google)
    <http://members.fortunecity.com/nnqweb/> (newusers)

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Dec 23, 2008
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Newbie fgets problem

    , Nov 2, 2004, in forum: C Programming
    Replies:
    8
    Views:
    575
    Old Wolf
    Nov 2, 2004
  2. Emerson

    problem with fgets

    Emerson, Dec 29, 2004, in forum: C Programming
    Replies:
    7
    Views:
    327
  3. Trond Valen
    Replies:
    5
    Views:
    446
    Niklas Norrthon
    Dec 7, 2005
  4. FireHead

    Replacing fgets Problem solved.

    FireHead, Apr 14, 2008, in forum: C Programming
    Replies:
    3
    Views:
    536
    Peter Nilsson
    Apr 14, 2008
  5. Jeff
    Replies:
    16
    Views:
    1,493
    Kenny McCormack
    Jun 9, 2008
Loading...

Share This Page