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!

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

Code:

   if(((int)*last) == '\n'){

Why the cast to int?

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

Originally Posted by Dino

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.

Originally Posted by matsp

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...

Originally Posted by matsp

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!

Originally Posted by Jesdisciple

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 *.

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.

Originally Posted by tabstop

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.

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.

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 */

Erm... Can someone tell me what I did that no one responded?

Originally Posted by Jesdisciple

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.

Originally Posted by citizen

Be patient. Perhaps we were working on the problem.

Woops. I'm experienced in forums, but everyone seemed to have left.

Originally Posted by citizen

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.

Originally Posted by citizen

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.

Originally Posted by citizen

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.

>> 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.

Originally Posted by Jesdisciple

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]