LINE_MAX is undefined error

[vedro-compota]

hi guys)
please tell me - why the Turbo C show the error -

LINE_MAX is undefined

for code =

/* */
#include <stdio.h> 
#include <alloc.h>/* */
#include <stdlib.h>
#include <limits.h>

 main()
{ 
  char a='';  
  char * ch;  
  char * inputstr; 
  printf("\nPlease specify the input string \n"); /
  fgets ( inputstr, LINE_MAX , stdin ); 
                                   
  printf("\n You've specified string = %s\n", inputstr); 
 

 scanf("%s", ch); 
 scanf("%s", ch);
 
}

big thanks in advance)

[Narue]

The compiler is right, LINE_MAX doesn't exist unless it's a library extension. I'd suggest the standard BUFSIZ, but that still won't work either because your code is severely broken. You're working with uninitialized pointers.

[Duoas]

LINE_MAX is an IEEE extension to the standard <limits.h>, found on some *nix systems. TurboC does is older than, and does not follow, Open Group specifications.

The proper way to do what you want is to use a stack-local variable:

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

#define MY_MAX 1000

int main()
{
  char inputstr[ MY_MAX ];
  printf( "%s", "\nPlease enter your input string\n" );

  /* Get your input string */
  fgets( inputstr, MY_MAX, stdin );

  /* Get rid of the trailing newline, if any.
     (Notice that this doesn't consider the 
     possibility that the user input a string
     larger than your available buffer. */
  char* newline = strchr( inputstr, '\n' );
  if (newline) *newline = '\0';

  printf( "%s%s%s", "Good job! You entered \"", inputstr, "\"!\n" );

  return 0;
}

BUFSIZ is declared in <stdio.h> if you want to use that, but it doesn't really correspond with what you are doing.

The problem with C I/O is that the only safe method is fgets(), but it is a pain to work with directly. There exists the GNU readline library, which helps you with this very easily (and gives the user input editing powers), but if you need you can roll your own simple version, like I did here.

Other issues:

The character '' doesn't exist. You must specify a value between the quotes. If you want the null character, use '\0' .

Unless you are doing something very specific, you don't need non-standard headers like <alloc.h>. Stick with the normal C90 standard libraries.

Those two lonely scanf() calls are both unnecessary and dangerous. If you must put stuff like at the end of your program (you shouldn't!), then use getchar().

Good luck!

[vedro-compota]

big thanks to you , Narue & Duoas )

Those two lonely scanf() calls are both unnecessary and dangerous. If you must put stuff like at the end of your program (you shouldn't!), then use getchar()

but how else can i stop the execotion process to see the result?)

[vedro-compota]

Why shouldn't i use the getchar() ? is this dangerous too?
Narue , my compiler doesn't think that

char a='';

is error....

[Narue]

Why shouldn't i use the getchar() ? is this dangerous too?

getchar() is the recommended way to pause before termination:

#include <stdio.h>

void discard_line(FILE *in)
{
    int ch;

    clearerr(stdin); /* We might be in an error state prior to the call */

    do
        ch = getchar();
    while (ch != '\n' && ch != EOF);

    clearerr(stdin); /* Reaching an EOF state is possible */
}

void pause_execution(void)
{
    discard_line(stdin);
    fputs("Press Enter to continue...", stdout);
    fflush(stdout);
    getchar();
}

int main(void)
{
    /* ... */

    pause_execution();

    return 0;
}

Narue , my compiler doesn't think that

char a='';

is error....

Your compiler neglects several errors in that code, it doesn't mean they're not errors. Turbo C is an exceptionally old compiler. It supports C89 to a decent extent, but you're likely to encounter some frustrating limitations due to the 16-bit architecture (on top of limitations from C's evolution since then). As such, my recommendation is to drop that compiler and get a newer one.

[vedro-compota]

which free one do you advise ,Narue ? (please tell for windows....because i'm not linux-star))

[Narue]

Visual C++ 2010 Express, Code::Blocks (MinGW's gcc), and Pelles C are all good.

[vedro-compota]

Narue , thank you one more time!)

[vedro-compota]

Duoas advised me to use this code =

char* simple_readline( const char* prompt, unsigned block_size )
  {
  char* result;   /* The malloc()ed string to return */
  char* block;    /* Each chunk of the string */
  char* ok;       /* Used to check for input errors */
  unsigned size;  /* The current size of the entire string */
  unsigned n;     /* strlen() of the last block */

  if (!block_size) block_size = 50;

  if (prompt) printf( "%s", prompt );

  block = result = (char*)malloc( size = block_size );
  if (!result) return NULL;
  *result = '\0';

  for (;;)
    {
    /* Get the next block of characters until EOL */
    ok = fgets( block, block_size, stdin );
    if (!ok) break;

    /* If we found EOL we are done */
    n = strlen( block );
    if (block[ n -1 ] == '\n') break;

    /* Else add more memory and try again */
    ok = (char*)realloc( result, size += block_size );
    if (!ok) break;

   [B] block  = ok +block -result -1;[/B]
    result = ok;
    }

[B]  if (!(*result))
    {
    free( result );
    result = NULL;
    }[/B]

  return result;
  }

But why he use the parts of code, which are in bold?
especially this (may be it's bug) =

[B] block  = ok +block -result -1;[/B]

If the result is null - will it be possible for *result part to be null (on the assumption of the previous code) ? (why do we need
next piece ) =

[B]  if (!(*result))
    {
    free( result );
    result = NULL;
    }[/B]

[Narue]

especially this (may be it's bug) =

block  = ok +block -result -1;

Looks like a bug to me. Addition of pointers is both nonsensical and illegal in C. I see what the author was going for, but it seems especially awkward. I'd rather just keep a running count of the string length and set block to the current null character on each iteration. It's much simpler that way:

char *simple_readline(void)
{
    const size_t block_size = 5;
    
    char *result, *block, *temp;
    size_t size = block_size;
    size_t n = 0;

    result = block = (char*)malloc(size);
    
    if (!result)
        return NULL;
    
    *result = '\0';

    for (;;) {
        if (fgets(block, block_size, stdin) == NULL)
            break;

        n += strlen(block);
        
        if (result[n - 1] == '\n')
            break;

        size += block_size;
        temp = (char*)realloc(result, size);
        
        if (temp == NULL)
            break;

        result = temp;
        block = result + n;
    }

    if (result[0] == '\0') {
        free(result);
        result = NULL;
    }

    return result;
}

If the result is null

result will never be null on the bolded lines. Prior to the loop, result is initialized using malloc(), and there's a test for failure which returns from the function. Inside the loop, result is never reset if realloc() fails.

(why do we need
next piece ) =

if (!(*result))
{
    free( result );
    result = NULL;
}

If result[0] is '\0' then that means the first call to fgets() failed. Rather than risk that case being mistaken as any kind of success (a successful call will always be a non-empty string), the author chose to free memory from the initial malloc() and return NULL.

[vedro-compota]

Looks like a bug to me. Addition of pointers is both nonsensical and illegal in C. I see what the author was going for, but it seems especially awkward. I'd rather just keep a running count of the string length and set block to the current null character on each iteration. It's much simpler that way:

char *simple_readline(void)
{
    const size_t block_size = 5;
    
    char *result, *block, *temp;
    size_t size = block_size;
    size_t n = 0;

    result = block = (char*)malloc(size);
    
    if (!result)
        return NULL;
    
    *result = '\0';

    for (;;) {
        if (fgets(block, block_size, stdin) == NULL)
            break;

        n += strlen(block);
        
        if (result[n - 1] == '\n')
            break;

        size += block_size;
        temp = (char*)realloc(result, size);
        
        if (temp == NULL)
            break;

        result = temp;
        block = result + n;
    }

    if (result[0] == '\0') {
        free(result);
        result = NULL;
    }

    return result;
}

result will never be null on the bolded lines. Prior to the loop, result is initialized using malloc(), and there's a test for failure which returns from the function. Inside the loop, result is never reset if realloc() fails.

If result[0] is '\0' then that means the first call to fgets() failed. Rather than risk that case being mistaken as any kind of success (a successful call will always be a non-empty string), the author chose to free memory from the initial malloc() and return NULL.

Narue, your example is exellent , but i compare it with the previous code and not understand all moments.

as i see - this original auther code won't work at all (even without bug about which line we spoke earlier =

block  = ok +block -result -1;

)
because of this line is destroing line , wich was read in previous iteration -

ok = fgets( block, block_size, stdin );

i thing so because this function (simple_readline) won't work correctly if the buffer size (block size) is smaller than length of input line.
Where i'm wrong?
there original function here(without one line) -

char* simple_readline( const char* prompt, unsigned block_size )
  {
  char* result;   /* The malloc()ed string to return */
  char* block;    /* Each chunk of the string */
  char* ok;       /* Used to check for input errors */
  unsigned size;  /* The current size of the entire string */
  unsigned n;     /* strlen() of the last block */

  if (!block_size) block_size = 50;

  if (prompt) printf( "%s", prompt );

  block = result = (char*)malloc( size = block_size );
  if (!result) return NULL;
  *result = '\0';

  for (;;)
    {
    /* Get the next block of characters until EOL */
    ok = fgets( block, block_size, stdin );
    if (!ok) break;

    /* If we found EOL we are done */
    n = strlen( block );
    if (block[ n -1 ] == '\n') break;

    /* Else add more memory and try again */
    ok = (char*)realloc( result, size += block_size );
    if (!ok) break;
    result = ok;
    }

  if (!(*result))
    {
    free( result );
    result = NULL;
    }

  return result;
  }

[Narue]

as i see - this original auther code won't work at all (even without bug about which line we spoke earlier =

You're correct. While addition of pointers is illegal, even if you fixed it, there's still a logic error in that line.

[vedro-compota]

ok . as i see the dimension of block_size doesn't influence on function execution speed. am i right?

[Narue]

as i see the dimension of block_size doesn't influence on function execution speed. am i right?

The block size determines how many allocations are made. If the block size is too small, you'll call realloc() an excessive number of times. If the block size is too big, you'll waste memory. The idea is to choose a block size that minimizes both realloc() calls and memory usage. Since dynamic allocation is a relatively expensive operation, block size most certainly influences the performance of the function.

[vedro-compota]

hm it's clear now) so it makes sense to send a block_size as parameter into the function. but if we allocated memory for next 10 character , but had read only 5 ones - what will we have in memory for last 5 characters in *result after returning to the main() func or we won't have this not occupied memory at all in *result ?

[Narue]

so it makes sense to send a block_size as parameter into the function

In my opinion, no. The block size is an implementation detail that shouldn't affect the interface. Rather, simple_getline() should choose one of the several automatic growth methods commonly used in string libraries.

but if we allocated memory for next 10 character , but had read only 5 ones - what will we have in memory for last 5 characters in *result after returning to the main() func or we won't have this not occupied memory at all in *result ?

The memory will still be there, just unused because fgets() properly terminates the string with '\0'. It's no different from typing 5 characters where the buffer is an array of 10.

[vedro-compota]

so will unused memory contain null characters?

[Narue]

so will unused memory contain null characters?

Not when allocated by malloc(). The unused memory will remain uninitialized.

[vedro-compota]

so when i return *result - will it length contain uninitialized memory or all characters will be real and the last one'll be = null ?
you've said that "the memory will still be there" but - what about this situation -
i've allocated memory for 15 characters, and I get from input stream only 10 symbols the last one is null (fgets() set it so) , assume that i return the result variable and somewhere in code i want to get value result[12] - will this value be from result memory area?

[Narue]

It's no different from if you used an array. Say the array has 10 characters and you type "abcd". The array will contain those four characters, plus a newline and a null character, which means the last four elements will be garbage unless you went out of your way to initialize them:

#include <ctype.h>
#include <stdio.h>

int main(void)
{
    char buf[10];
    size_t i;
    
    /* Initialize to predictable garbage */
    for (i = 0; i < 10; i++)
        buf[i] = '~';
    
    printf("Enter 4 characters: ");
    fflush(stdout);
    
    if (fgets(buf, sizeof buf, stdin) != NULL) {
        for (i = 0; i < sizeof buf; i++) {
            if (isprint(buf[i]))
                printf("'%c'\n", buf[i]);
            else
                printf("[%d]\n", buf[i]);
        }
    }
    
    return 0;
}

Your memory will work the same way with malloc().

[Duoas]

Looks like a bug to me. Addition of pointers is both nonsensical and illegal in C. I see what the author was going for, but it seems especially awkward. I'd rather just keep a running count of the string length and set block to the current null character on each iteration. It's much simpler that way:

You're correct. While addition of pointers is illegal, even if you fixed it, there's still a logic error in that line.

Pointer arithmetic is not illegal. What is illegal is to perform arithmetic on unrelated pieces of memory. The pointer arithmetic was not a mistake; however I could have been more careful to code it as:

block  = ok + (size_t)(block - result) - 1;

Since you are in to giving a code review, I'd sure like to know exactly what the logic error is in that line. It seemed me some pretty simple addition for me that kind of error.

Also, I don't think that there is any point in keeping an extra variable around to track information that is already available in (block - result) . If I were developing under some specific company guidelines that addressed stuff like that I could easily adjust to do just as you would. Otherwise it comes down to nothing more than simple, personal preference.

as i see - this original auther code won't work at all (even without bug about which line we spoke earlier ...

Those are pretty careless statements to make, considering you don't understand the algorithm. The routine has no bug -- at least not that I or anyone else has actually been able to discover so far, and it works just fine -- which you would know if you had done so much as to compile it and try it.

No data is "destroyed" (or memory "leaked") because the realloc() function cleans up the previous allocation for you.

As I have already explained to you in PM, the way this is done is simply to keep a buffer of characters. If the user tries to enter more characters than the buffer can handle, the buffer is resized (via realloc() ), and an additional fgets() is attempted. This is done as many times as necessary to get all the user's input.

It would destroy data already received from the user if we just read into the beginning result again, so the block pointer references the next available character in result to put the user's input.

In my opinion, no. The block size is an implementation detail that shouldn't affect the interface. Rather, simple_getline() should choose one of the several automatic growth methods commonly used in string libraries.

That is waaay beyond a simple readline.

I wonder, however, why the OP is obsessing over this instead of just trying the GNU Readline library -- seeing as it is such an issue to use it? It is significantly more powerful (and convoluted) than my simple version. The only thing I didn't do that I would now is get rid of the newline at the end of the completed input.

If it is too much for you to deal with -- then don't. The routine is pretty straight-forward, and by doing all the weird stuff in one spot you are relieved from having to do it everywhere you get input in your code.

[Narue]

Pointer arithmetic is not illegal. What is illegal is to perform arithmetic on unrelated pieces of memory. The pointer arithmetic was not a mistake; however I could have been more careful to code it as:

Go read the standard, dude. Addition of two pointers is not legal. Here, let me quote it for you: "For addition, either both operands shall have arithmetic type, or one operand shall be a pointer to an object type and the other shall have integer type." The code as originally posted attempts to add two pointers ( ok + block ), and thus is wrong.

If you wrote something different and it was translated incorrectly, that's not my problem; I'm commenting only on the code in this thread.

That is waaay beyond a simple readline.

Bullshit. That's about as basic as it gets for reading arbitrary length strings. Don't flatter yourself; any programmer with half a brain and marginal experience in C could easily write such a function, and probably has more than once. You're not special because you managed to resize a dynamic block of memory in the least imaginative way possible.

It's fine to boast an ego for fun, but don't let it get in the way of progress. Your post reeks of a bruised ego and childish behavior in the face of criticism. Nobody will think less of you if you make a mistake, but they most certainly will if you act like a brat after it's pointed out.

[Duoas]

It is not egotistic to simply respond to having one's very simple code roundly abused. The function has the performance and memory characteristics desired when I wrote it. It has no leaks. It does what it is supposed to do. And it isn't filled with convoluted, non-standard stuff.

I've responded to the syntax error you have actually shown me by correcting it, but you have yet to respond by showing exactly how my function has a "logic error" or what all these unnamed "bug"s are. Instead you have resorted to ad homenim by personally belittling my understanding, experience, imagination, and behavior.

Bye.

[vedro-compota]

guys stop swearing! both of you are helping other people. and that's great!