[Techtalk] Re: [Newchix] learning c

[Devdas Bhagat]

On 25/03/04 11:53 -0800, Alice Moomlyn wrote:
<snip bit about being a C newbie>
This should go to techtalk. Not newchix. Followups set.

> 1. if I've written code which will be really
> inefficient.
> 
> 2. if I've done something the hard way when there was
> a library function which I could have used.`
> 
> 3. if my code is ugly, unreadable, badly commented or
> generally lacking in style.
> 
> 4. if there are bugs that I didn't spot.
> 
> 
> Here's my first program.
> 
> It reads a string from input that may be of *any*
> length (less than 20470 characters).
> 
> Not very exciting, I know, but its the kind of thing I
> miss from python :^P
> 
> P.S I'm using yahoo mail, so if its really badly
> formatted, that'd be why. Sorry. 8^<
> ..................................................
/* 
 * C style comments follow
 */
/*
 * Handling strings in C is hard to get correct. The edge cases are the
 * worst, and failure to check those usually leads to those nasty buffer
 * overflow bugs.
 */
> 
> #include <stdio.h>
> #include <string.h> 
> #include <stdlib.h>

/* 
 * C programmers generally do not use a boolean type.
 * If you really want a boolean, use enum instead.
 */
/* 
 * #defined terms are generally CAPITALIZED. This is not necessary, but
 * is a convention for simple constants. Macros which replace functions
 * are almost always lowercase.
 */
> #define bool int
> #define True 1
> #define False 0
> 
/*
 * Comments precede the function. If I am reading code, I want to know
 * what that code is supposed to do before I start looking at it.
 */
> char *alice_getstring();
> /* returns a pointer to a string read from standard input,
>    of maximum length 20470 characters. */
> 
> bool alice_endline(char *string, int size);
> /* searches through a string for the newline character,
>    returns the number of characters searched if found,
>    or False if not found. */
> 
/*
 * int main(void) just to be explicit in your code
 */
> int main()
> {
>    fputs("Enter a string of any length: ", stdout);
/*
 * You need to declare variables before invoking any code. Also, you
 * have not allocated any memory for s. Have you even compiled and run
 * this program?
 */
>    char *s = alice_getstring();
>    fputs("\nThis is the string you entered:", stdout);
>    fputs(s, stdout);
> 
>    return 0;
> }
> 
> 
> char *alice_getstring()
> {
>    int buffsize = 10;
>    int maxbuffers = 10;
>    int totalbuff = 0;
> 
>    char *p[maxbuffers]; /* an array of pointers to character strings */
> 
/*
 * Shouldn't this comment be before the function declaration?
 */
>    /* Begins by reading into a buffer of size 'buffsize'.
>       If no newline encountered, creates a new buffer of size 2*buffsize
>       and reads into that. If still no newline encountered,
>       creates a buffer of size 4*buffsize and reads into that, etc....
>       Each new buffer is twice the size of the last one,
>       and the maximum number of buffers is 'maxbuffers'   */
>       
>    int endline; /* stores number of position actually used 
>                    to store input in the final buffer */
> 
>    int counter = 0;
>    bool finished = False;
>    while (finished == False)
>    {
>       p[counter] = malloc(buffsize*sizeof(char)); 
/*
 * What happens if malloc() fails?
 */
>       p[counter] = fgets(p[counter], buffsize*sizeof(char), stdin); 
/*
 * Whitespace around those brackets. Whitespace is good.
 */
/*
 * What happens if I give a larger input to this function than your
 * arbitrary size limit? Should you not check that buffsize does not
 * exceed some fixed limit and error out?
 */
>       endline = (alice_endline(p[counter],buffsize));
>       if (endline == False) /* still more input */
>       {
>          totalbuff += buffsize;
>          buffsize *= 2;
>          counter++;
>       }
>       else /* end of input */
>       {
>          totalbuff += endline; /* the last buffer will probably not be full */
/*
 * Are you sure your buffer space is not really and truly full? Probably
 * is not good enough.
 */
>          finished = True;
>       }
>    };
> 
/*
 * Why would you not use realloc here? That is exactly why it exists.
 */
>    /* combine all the buffers into one */
>   
>    char *result = malloc(totalbuff*sizeof(char));
> 
>    buffsize = 10;
>    result = strncpy(result, p[0], buffsize);
> 
>    if (counter == 0)
>       return result;
> 
/*
 * This is not C++ or Python. Variables have to be declared at the
 * beginning of the function declaration.
 */
>    int loopvar = 1;
>    while (loopvar < counter)
>    {
>        buffsize *= 2;
>        result = strncat(result, p[loopvar], buffsize);
>        free(p[loopvar]);
>        loopvar++;
>    }
>    result = strncat(result, p[loopvar], endline);
>   
>    return result;
> }
> 
> bool alice_endline(char *string, int size)
> {
>     int counter = 0;
>     while (counter < size)
>     {
>        if (string[counter] == '\n')
>           return (counter + 1);
>        else
>           counter++;
>     };
> 
>     return False;  /* newline not found */
>    
> }

Devdas Bhagat