fgets only returns 3 characters

This is a discussion on fgets only returns 3 characters within the C Programming forums, part of the General Programming Boards category; This doesn't seem to be specific to any file; I tried it with an ordered list and once with "blah" ...

  1. #1
    Registered User
    Join Date
    Aug 2008
    Posts
    129

    fgets only returns 3 characters

    This doesn't seem to be specific to any file; I tried it with an ordered list and once with "blah" in a file by itself. I got a count of 3 characters each time (after noticing only 3 characters and changing the output to their number instead).

    I'm sure it's something uber-simple and so would appreciate help all the more. EDIT: I get a segfault when trying to output the string- or array-length of string in readLine...

    EDIT2: OK, I figured out that my array's length isn't accessible to me after all... Passing the length separately fixes the problem.
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    
    void readLine(char* string, FILE* stream){
        int i, length = sizeof(string);
        
        fgets(string, length, stream);
        char* last = &string[strlen(string) - 1];
        if(((int)*last) == '\n'){
            *last = 0;
        }
        return;
    }
    
    void concat(char* result, void (*callback)(char* result), char* array[]){
        int i, sum = 0, length = sizeof(array);
        for(i = 0; i < length; i++){
            sum += strlen(array[i]);
        }
        result = calloc(sum, sizeof(char));
        for(i = 0; i < length; i++){
            if(array[i]){
                strcat(result, array[i]);
            }
        }
        callback(result);
        free(result);
    }
    
    void interrupt(char* error){
        fwrite(error, strlen(error), 1, stderr);
    }
    
    int main(int argc, char* argv[]){
        if(argc < 2){
            char* error = "Error: At least one argument (a filename) must be provided.";
            fwrite(error, strlen(error), 1, stderr);
            return 1;
        }
        char mode = 'r';
        FILE* file = fopen(argv[1], &mode);
        if(!file){
            char* array[3];
            array[0] = "Error: The file \"";
            array[1] = argv[1];
            array[2] = "\" could not be opened; please check its permissions and try again.";
            char* error;
            concat(error, &interrupt, array);
            return 1;
        }
        char input[1024];
        readLine(input, file);
        printf("&#37;d", strlen(input));
        return 0;
    }
    Thanks!
    Last edited by Jesdisciple; 08-21-2008 at 12:15 AM.

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    That is because sizeof(string) is 4 [1] - and that means three characters + terminating zero.

    You need to use sizeof() at the level outside of readLine - this is what I tried to say in the previous thread.

    [1] Or 2, 8 or some other number that is sizeof(char *) for the current compiler (and can vary with settings in the compiler too).

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    Code:
       if(((int)*last) == '\n'){
    Why the cast to int?
    Mac and Windows cross platform programmer. Ruby lover.

    Quote of the Day
    12/20: Mario F.:I never was, am not, and never will be, one to shut up in the face of something I think is fundamentally wrong.

    Amen brother!

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Picking some more nit:
    Code:
    void concat(char* result, void (*callback)(char* result), char* array[]){
    No point in passing result to the concat function - it is only usable within the function, since:
    1. You never will see the new value of result that you have in the function - because you are just passing the pointer by value.
    2. You free the pointer at the end of the function.
    So you may just as well have a local variable.

    Inside the concat function:
    Code:
    length = sizeof(array);
    This, just like the first piece of code, is incorrect - you either need to mark the end of the array with a NULL ponter [make it one larger], or pass the number of entries to the function.

    Code:
        result = calloc(sum, sizeof(char));
    Should be:
    Code:
        result = calloc(sum+1, sizeof(char));
    to account for the terminating zero.

    Although it's pretty pointless to use calloc, as you are just about to overwrite the entire array with strings. malloc(sum+1) would do just as well.

    Whilst it's technically correct:
    Code:
        fwrite(error, strlen(error), 1, stderr);
    I would prefer the more tradtional:
    Code:
        fprintf(stderr, "%s", error);
    or
    Code:
        fputs(error, stderr);
    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Quote Originally Posted by Dino View Post
    Code:
       if(((int)*last) == '\n'){
    Why the cast to int?
    I was getting a warning about incompatible types, but I don't see it now so I've removed the cast.

    Quote Originally Posted by matsp View Post
    You need to use sizeof() at the level outside of readLine - this is what I tried to say in the previous thread.
    Woops... You did say that, now that I look back, but I missed it.

    Mats, your points are all satisfied except one...

    Quote Originally Posted by matsp View Post
    Although it's pretty pointless to use calloc, as you are just about to overwrite the entire array with strings. malloc(sum+1) would do just as well.
    See http://irc.essex.ac.uk/www.iota-six....at_strncat.asp for why I used calloc.

    Now to ask the question I (sort of) intended in the other thread: Why is the length of an array not accessible in every scope as it is in the declaring scope?

    Thanks!
    Last edited by Jesdisciple; 08-21-2008 at 10:49 AM.

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Quote Originally Posted by Jesdisciple View Post
    Now to ask the question I (sort of) intended in the other thread: Why is the length of an array not accessible in every scope as it is in the declaring scope?

    Thanks!
    When you type "arrayname" by itself, that name "decays" into a pointer to the start of the array (this happens everywhere, not just functions) so the function is only going to see a char *.

  7. #7
    Registered User whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    7,667
    You cannot calculate the length of an array in another scope using sizeof because it got converted to a pointer to the first element. For example:
    Code:
    #include <stddef.h>
    
    void foo (int * elems, size_t count);
    
    int main()
    {
      int arr[3];
      /** can compute length of arr now **/
      size_t count = sizeof arr / sizeof arr[0];
      foo(arr, count);
      
      /** ... **/
      return 0;
    }
    You see? Now, if you haven't kept track of the length of your arrays, you would need the help of an external function to calculate their length. Think of strings for example. All strlen does is count the number of elements it passed over until it finds a terminating zero.

  8. #8
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by tabstop View Post
    When you type "arrayname" by itself, that name "decays" into a pointer to the start of the array (this happens everywhere, not just functions) so the function is only going to see a char *.
    That is correct. but in the scope where it's declared, sizeof() will know the size of the array, but it won't give the correct result outside of that function.
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  9. #9
    Banned master5001's Avatar
    Join Date
    Aug 2001
    Location
    Visalia, CA, USA
    Posts
    3,685
    All those pesky functions that require you to put in an array's size as a parameter do so for a reason. Unless you make a function that specifically states that it requires an array of x bytes in its documentation (which is NOT a good programming practice to begin with), you will always have to specify an array size. Its just a troubling fact of life in programming. The one upshot to that is the fact that if there is a buffer overflow, you can blame the person who wrote the code that calls the function. I guess every cloud does have a silver lining.

  10. #10
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    I guess I'm expecting arrays to have a natively object-oriented implementation, which is silly in a non-object-oriented language. I have three other related questions which I think merit other threads. But, since I was wrong last time, I'll just ask them here.

    When can I assign NULL? I'm trying to solve the problem of the array's length, but my dereferenced pointers don't like the NULL value. I have this problem on lines 29, 30, 31, and 37 of list.h.
    Code:
    error: incompatible types in assignment
    In a linked list, how can the implementation allow all types of values but also let the downstream program know what types it's handling? Not classic generics (although I would prefer it) but downstream type-checking. EDIT: BTW, main.c doesn't reflect this endeavor yet.

    Does C support throwing & catching exceptions rather than just printing to stderr?

    main.c
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>
    #include "list.h"
    
    void readLine(char* string, int length, FILE* stream){
        int i;
        
        fgets(string, length, stream);
        char* last = &string[strlen(string) - 1];
        if(*last == '\n'){
            *last = 0;
        }
        return;
    }
    
    void concat(char* array[], void (*callback)(char* result)){
        int i, sum = 0, length = sizeof(array);
        
        for(i = 0; i < length; i++){
            sum += strlen(array[i]);
        }
        char* result = calloc(sum + 1, sizeof(char));
        for(i = 0; i < length; i++){
            if(array[i]){
                strcat(result, array[i]);
            }
        }
        callback(result);
        free(result);
    }
    
    void interrupt(char* error){
        fputs(error, stderr);
    }
    
    int main(int argc, char* argv[]){
        if(argc < 2){
            char* error = "Error: At least one argument (a filename) must be provided.";
            fwrite(error, strlen(error), 1, stderr);
            return 1;
        }
        char mode = 'r';
        FILE* file = fopen(argv[1], &mode);
        if(!file){
            char* array[3];
            array[0] = "Error: The file \"";
            array[1] = argv[1];
            array[2] = "\" could not be opened; check its permissions and try again.";
            array[3] = NULL;
            concat(array, &interrupt);
            return 1;
        }
        char input[256];
        readLine(input, sizeof(input), file);
        printf("&#37;s", input);
        return 0;
    }
    list.h
    Code:
    /* 
     * File:   list.h
     * Author: Chris "Jesdisciple"
     *
     * Created on August 21, 2008, 12:47 PM
     */
    
    #ifndef _LIST_H
    #define	_LIST_H
    
    #ifdef	__cplusplus
    extern "C" {
    #endif
    
    #include "object.h"
    
    struct node{
        struct object* value;
        struct node* next;
        struct list* list;
    };
    struct list{
        struct node* root;
        struct node* current;
        struct node* last;
        int length;
    };
    void list(struct list* list){
        *(*list).root = NULL;
        *(*list).current = NULL;
        *(*list).last = NULL;
        (*list).length = 0;
    }
    void link(struct list* list, struct object* value){
        struct node node;
        *node.value = *value;
        *node.next = NULL;
        *node.list = *list;
        *(*(*list).last).next = node;
        *(*list).last = node;
        (*list).length++;
    }
    
    #ifdef	__cplusplus
    }
    #endif
    
    #endif	/* _LIST_H */
    object.h
    Code:
    /* 
     * File:   object.h
     * Author: Chris "Jesdisciple"
     *
     * Created on August 21, 2008, 1:14 PM
     */
    
    #ifndef _OBJECT_H
    #define	_OBJECT_H
    
    #ifdef	__cplusplus
    extern "C" {
    #endif
    
    struct object{
        int pointer;
        int length;
        int type;
    };
    
    #ifdef	__cplusplus
    }
    #endif
    
    #endif	/* _OBJECT_H */
    Last edited by Jesdisciple; 08-21-2008 at 01:36 PM.

  11. #11
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Erm... Can someone tell me what I did that no one responded?

  12. #12
    Registered User whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    7,667
    Quote Originally Posted by Jesdisciple View Post
    Erm... Can someone tell me what I did that no one responded?
    Be patient. Perhaps we were working on the problem.

    It looks like you're trying to make a flexible linked list that can hold any data. Solutions to this problem are very prevalent if you don't want to make one yourself, just search the web for a library.

    But you said the problem was here:
    Code:
    void link(struct list* list, struct object* value){
        struct node node;
        *node.value = *value;
        *node.next = NULL;
        *node.list = *list;
        *(*(*list).last).next = node;
        *(*list).last = node;
        (*list).length++;
    }
    I personally think you need to rewrite some of this library if you insist on doing so. You haven't documented the functions you use nor the types that they work with. I don't know why object contains an int named pointer, and other suspect things. It's difficult to know what's right with this library in general.

    NULL is defined as a pointer. Note that node is not declared a pointer (which is why NULL can't be assigned) and that any changes to node will not persist; the list will be broken after this function returns.

  13. #13
    Registered User
    Join Date
    Aug 2008
    Posts
    129
    Quote Originally Posted by citizen View Post
    Be patient. Perhaps we were working on the problem.
    Woops. I'm experienced in forums, but everyone seemed to have left.

    Quote Originally Posted by citizen View Post
    It looks like you're trying to make a flexible linked list that can hold any data. Solutions to this problem are very prevalent if you don't want to make one yourself, just search the web for a library.
    Partly... I also want to be able to check the type of any node's value in the calling/downstream code to make sure I know how to handle it.

    Quote Originally Posted by citizen View Post
    But you said the problem was here:
    Code:
    void link(struct list* list, struct object* value){
        struct node node;
        *node.value = *value;
        *node.next = NULL;
        *node.list = *list;
        *(*(*list).last).next = node;
        *(*list).last = node;
        (*list).length++;
    }
    I personally think you need to rewrite some of this library if you insist on doing so. You haven't documented the functions you use nor the types that they work with. I don't know why object contains an int named pointer, and other suspect things. It's difficult to know what's right with this library in general.
    pointer is an int because I don't know what type it points to, i.e. I can't make it a real pointer.

    Quote Originally Posted by citizen View Post
    NULL is defined as a pointer. Note that node is not declared a pointer (which is why NULL can't be assigned) and that any changes to node will not persist; the list will be broken after this function returns.
    Oh, NULL is a pointer? Well that clears that issue up. You're right that node isn't a pointer, but node.next is. But I was dereferencing it to assign NULL.

    To clarify my second question, I need the equivalent of typeof or instanceof in JavaScript; getClass or instanceof in Java; is_a() or instanceof in PHP. I found the GCC typeof extension, but I don't think even that will tell me at runtime what type something is.
    Last edited by Jesdisciple; 08-21-2008 at 08:19 PM.

  14. #14
    Registered User whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    7,667
    >> Partly... I also want to be able to check the type of any node's value

    You talked about this later, but C doesn't have run-time type identification because it is a procedural language. It seems like C++ would be a better choice for what you want to do, because it does have that, is more object oriented, and has templates, so you can create a list of any type. In fact, std::list would solve all of your problems already. But since this is the C board and you may still be interested in doing this:


    >> pointer is an int because I don't know what type it points to, i.e. I can't make it a real pointer.

    C has a type for generic pointers called "void*".

    Code:
    struct node {
      void * data;
      struct node * next;
      struct node * prev;
    };
    And then if I were implementing a library, I would separate list processing tasks into functions which don't care about what data really is. When the type of data matters (such as when you need to remove a node) I would have to rely on external functions to "do the right thing." It's a lot of work to do correctly and if you do rely on void*, it has some drawbacks you will need to code around. The most important IMO is that it makes the code less type strict. If you try to do something like merge two lists of different types, you won't have much standing in your way. You'll just break your lists.

    >> Oh, NULL is a pointer? Well that clears that issue up. You're right that node isn't a pointer, but
    >> node.next is. But I was dereferencing it to assign NULL.

    That's not the right way to do it for a compound type like node. node really should be a pointer if you want changes to the list to persist when the function ends.

    Since the dereferencing operation is supposed to happen before member access, you would need to type (*node).next or node->next. Dereferencing is below member access on the operator precedence table.
    Last edited by whiteflags; 08-21-2008 at 08:48 PM.

  15. #15
    Registered User
    Join Date
    Jan 2008
    Posts
    287
    Quote Originally Posted by Jesdisciple View Post
    But I was dereferencing it to assign NULL.
    You should NOT dereference a pointer if you intend to set it to NULL. When you dereference, you are instructing the compiler that you would like to modify the object that the pointer points to. If that pointer is pointing to an int, you don't want to set that int to NULL do you? You want to set the pointer to NULL.

    So in summary, if node.next is a pointer, then node.next = NULL sets it to NULL.

    [edit]
    Also, I'm not entirely sure that its correct to say that NULL is a pointer. A null pointer in C is defined as a constant zero integer value. When a 0 appears in pointer context, the compiler converts it to the appropriate internal null pointer value for the pointer type it is being assigned to (which may be different for different pointer types!).

    So, defining NULL to be 0 is perfectly valid. In fact, some people advocate not using NULL, and rather using 0 in its place. NULL, after all, is nothing more than a stylistic hint to the reader that the value should be interpreted as a pointer.

    Some compilers will define NULL by casting the 0 to a pointer value, like so: ((void*) 0). This may or may not be a good thing.
    [/edit]
    Last edited by arpsmack; 08-21-2008 at 08:54 PM.

Page 1 of 6 123456 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Lame null append cause buffer to crash
    By cmoo in forum C Programming
    Replies: 8
    Last Post: 12-29-2008, 02:27 AM
  2. HELP!!!!emergency Problem~expert please help
    By unknowppl in forum C++ Programming
    Replies: 9
    Last Post: 08-21-2008, 06:41 PM
  3. HELP!!!!emergency ~expert please help
    By unknowppl in forum C Programming
    Replies: 1
    Last Post: 08-19-2008, 07:35 AM
  4. extract characters to '.'
    By VanJay011379 in forum C++ Programming
    Replies: 5
    Last Post: 07-29-2002, 11:32 PM
  5. gets vs fgets
    By strobo in forum C Programming
    Replies: 10
    Last Post: 03-27-2002, 04:28 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21