Thread: Anyone able to help a beginner?

  1. #1
    Registered User
    Join Date
    Nov 2012
    Posts
    19

    Smile Anyone able to help a beginner?

    Hi,
    I am a beginner to C, and after learning some stuff thought I'd try and write a simple adventure game. I haven't done much of it yet, only basic interaction, but am wondering how to improve the code so far (especially memory management!). It is on github here:
    https://github.com/Joshun/adventure/

    This is my first attempt at making something, so don't mock me too much if it is terrible (likely)!

    Thanks

    Joshun

  2. #2
    Programming Wraith GReaper's Avatar
    Join Date
    Apr 2009
    Location
    Greece
    Posts
    2,739
    Always free the memory when you no longer need it. Here, it may pose no problem at all, but it will come to haunt you if you try to reuse the code in a different way.
    Devoted my life to programming...

  3. #3
    Registered User
    Join Date
    Nov 2012
    Posts
    19

    Thumbs up How do I fix this?

    Thanks for the tip. I wanted to free the memory (when the program ends, just before
    Code:
    return 0
    ), but it just segfaults - any idea how to resolve this? This is on a Linux-based operating system.
    Code from adventure.c:
    Code:
    // Text Adventure v1 by Joshua O'Leary
    
    
    // Preprocessor Directives
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include "definitions.h"
    
    
    // Debug Mode when set to 1
    #define DEBUG 1
    
    
    #define INPUTLEN 20
    
    
    #define PROMPT "\nWhat now?"
    //extern struct gameplay status;
    
    
    //void initial_state()
    //{
    //    status.gameover = 0; status.room = 2;
    //}
    
    
    int main()
    {    
        //initial_state();
        char *input = malloc(INPUTLEN * sizeof(char));
        printf("\nAdventure v1");
        printf("\nYou are outside the house");
        while ( status.gameover == 0 )
        {
            prompt(&input);
        }
        return 0;
    }
    
    
    void control(char value)
    {
        switch (value)
        {
            case 'q':
            printf("\nExiting...");
            status.gameover = 1;
            break;
        }
    
    
    }
    
    
    void prompt(char *input)
    {    
        printf("%s", PROMPT);
        gets(input);
        //printf("%c", *input);
        //input++;
        //printf("%c", *input);
        analyse(&input);
    }
    
    
    void analyse(char **input)
    {
        #if DEBUG == 1
            printf("\nInput: %s", *input);
            printf("\nRoom: %d", status.room);
        #endif
        switch ( **input )
        {
            case 'Q':
            case 'q':
                printf("\nquit called");
                control('q');
                break;
            case 'N':
            case 'n':
                gotoroom(0);
                break;
            case 'S':
            case 's':
                gotoroom(1);
                break;
            case 'E':
            case 'e':
                gotoroom(2);
                break;
            case 'W':
            case 'w':
                gotoroom(3);
                break;
            default :
                printf("\n?!");
                break;
        }
    }
    rooms.c (kept separate because there may be long descriptions:
    Code:
    #include <stdio.h>
    #include "definitions.h"
    
    
    void gotoroom(int direction)
    {
    
    
        // Room Descriptions
        const char *descript[] = { "You are outside the house",
                                    "You are in the front room",
                                    "You are on the door-mat" };
    
    
        // Room Maps
        const int map[][4] = { { 1, -1, -1, -1 }, /* { north, south, east, west } */
                               { 2, 0, 3, 2 },    /*  a negative value means you can't move there */
                               { -1, 1, 1, 3 } };
        //printf("%d", map[status.room][direction]);
        //printf("%d", status.room);
        if ( map[status.room][direction] < 0 )
        {
            printf("\nYou can\'t go there");
        }
        else
        {
            status.room = map[status.room][direction];
            printf("\n%s", descript[status.room]);
        }
    }
    And of course definitions.h:
    Code:
    void control(char value);
    void prompt();
    void analyse(char **input);
    void gotoroom(int direction);
    
    
    struct gameplay
    {
        int room;
        int gameover;
        int hasitems[5];
    } status;

  4. #4
    SAMARAS std10093's Avatar
    Join Date
    Jan 2011
    Location
    Nice, France
    Posts
    2,694
    Code:
    prompt(&input);
    change the above to this
    Code:
    prompt(input);
    because input is already a char pointer

    Aslo welcome to the forum
    (which is not Greek despite the fact that two Greeks answered to you )

  5. #5
    Registered User
    Join Date
    Dec 2007
    Posts
    2,675
    Code:
    gets(input);
    No, no, no....a thousand times no!

    FAQ > Why gets() is bad / Buffer Overflows - Cprogramming.com

    Code:
    void analyse(char **input)
    There's absolutely no need to use char** here. Just pass a single char. Unless you're planning on expanding this to use more than one char, in which case it should be

    Code:
    void analyse(const char *input)
    Last edited by rags_to_riches; 11-23-2012 at 04:50 PM.

  6. #6
    Registered User
    Join Date
    Nov 2012
    Posts
    19
    Thanks std10093, that cleared things up. Those pesky pointers, it is always hard to remember the input format especially with char pointers (which sometimes require the addressof operator). Thanks for welcoming me too

    Apart from that mistake, is there anything else that could be unsafe in my code? I assumed that char pointers would be better for user input than char arrays (overflow can cause segfault), just wondering whether it is a good practice.

    Joshun

  7. #7
    Registered User
    Join Date
    Nov 2012
    Posts
    19
    Sorry rags_to_riches, you posted before I refreshed. The analyse function does require access to more than one char when more complicated command words are added later. I've managed to sort that out and changed analyse to
    Code:
    void analyse(char *input)
    Const char * doesn't work though, it doesn't seem to allow fgets to access the pointer.

    I have however changed
    Code:
    gets(input);
    to
    Code:
    fgets(input, INPUTLEN, stdin);
    , which works as expected.
    Last edited by Joshun; 11-23-2012 at 05:25 PM.

  8. #8
    Stoned Witch Barney McGrew's Avatar
    Join Date
    Oct 2012
    Location
    astaylea
    Posts
    420
    You should check the return value of both fgets and malloc, and handle the case where they return null pointers appropriately. Also, it isn't necessary to multiply INPUT_LEN by sizeof (char), because sizeof (char) is guaranteed to be 1.

    Although it's not such a huge issue, you ought to put the '\n' character at the end of a line rather than the beginning. Functions like puts work this way, and there's an issue with some C implementations that require text streams to have a terminating newline character on their last line.

  9. #9
    Registered User
    Join Date
    Nov 2012
    Posts
    19

    Thumbs up

    Thanks Barney McGrew for the advice. The program now exits if there is not sufficient memory (though incredibly unlikely) and I have removed sizeof(char) since it is needed. I'm not too worried about the \n issue as the C implementations I recommend work fine with it - the makefile requires gcc anyway. The current code now:
    https://github.com/Joshun/adventure

    improved adventure.c:
    Code:
    // Text Adventure v1 by Joshua O'Leary
    
    
    // Preprocessor Directives
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include "definitions.h"
    
    
    // Debug Mode when set to 1
    #define DEBUG 1
    
    
    #define INPUTLEN 20
    
    
    #define PROMPT "\nWhat now? "
    
    
    int main()
    {    
        //char *input = malloc(INPUTLEN * sizeof(char));
        char *input = malloc(INPUTLEN);
        if ( input == NULL ) { printf("Insufficient memory. Exiting...\n"); return 1; }
        printf("\nAdventure v1");
        printf("\nYou are outside the house");
        while ( status.gameover == 0 )
        {
            prompt(input);
        }
        free(input);
        return 0;
    }
    
    
    void control(char value)
    {
        switch (value)
        {
            case 'q':
            printf("\nExiting...\n");
            status.gameover = 1;
            break;
        }
    
    
    }
    
    
    void prompt(char *input)
    {    
        printf("%s", PROMPT);
        //gets(input);
        fgets(input, INPUTLEN, stdin);
        //printf("%c", *input);
        //input++;
        //printf("%c", *input);
        analyse(input);
    }
    
    
    void analyse(const char *input) /* char * for single char */
    {
        #if DEBUG == 1
            printf("\nInput: %s", input);
            printf("\nRoom: %d", status.room);
        #endif
        switch ( *input )
        {
            case 'Q':
            case 'q':
                printf("\nquit called");
                control('q');
                break;
            case 'N':
            case 'n':
                gotoroom(0);
                break;
            case 'S':
            case 's':
                gotoroom(1);
                break;
            case 'E':
            case 'e':
                gotoroom(2);
                break;
            case 'W':
            case 'w':
                gotoroom(3);
                break;
            default :
                printf("\n?!");
                break;
        }
    }

  10. #10
    Registered User
    Join Date
    Sep 2012
    Posts
    357
    For your program to behave as all standard unix utilities, always print a '\n' at the end of the string (not at the beginning)

    Code:
    /* ... */
        printf("\nAdventure v1");
        printf("\nYou are outside the house");
    /* ... */
            printf("\nExiting...\n");
    /* ... */
                printf("\nquit called");
    /* ... */
    Code:
    /* ... */
        printf("Adventure v1\n");
        printf("You are outside the house\n");
    /* ... */
            printf("Exiting...\n\n");
    /* ... */
                printf("quit called\n");
    /* ... */

  11. #11
    Registered User
    Join Date
    Nov 2012
    Posts
    19

    Smile

    Ok thanks, I have finally sorted out the issues with the \n's. I have also added a function to interpret a 'take' command, which copies the first 5 characters after the word 'take ' from the prompt variable. The improved code:

    Code:
    // Text Adventure v1 by Joshua O'Leary
    
    
    // Preprocessor Directives
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #include "definitions.h"
    
    
    // Debug Mode when set to 1
    #define DEBUG 1
    
    
    #define INPUTLEN 20
    
    
    #define PROMPT "What now? "
    
    
    int main()
    {    
        //char *input = malloc(INPUTLEN * sizeof(char));
        char *input = malloc(INPUTLEN); if ( input == NULL ) { printf("Insufficient memory. Exiting...\n"); return 1; }
        printf("Adventure v1\n");
        printf("You are outside the house\n");
        while ( status.gameover == 0 )
        {
            prompt(input);
        }
        free(input);
        return 0;
    }
    
    
    void control(char value)
    {
        switch (value)
        {
            case 'q':
            printf("Exiting...\n");
            status.gameover = 1;
            break;
        }
    
    
    }
    
    
    void buffcheck(char *buffer)
    {
         if ( buffer == NULL ) { printf ("Warning: Out of memory!\n"); }
    }
    
    
    void prompt(char *input)
    {    
        printf("%s", PROMPT);
        //gets(input);
        fgets(input, INPUTLEN, stdin);
        //printf("%c", *input);
        //input++;
        //printf("%c", *input);
        analyse(input);
    }
    
    
    void analyse(const char *input) /* char * for single char */
    {
        char *buffer = malloc(4); buffcheck(buffer);
        #if DEBUG == 1
            printf("Input: %s\n", input);
            printf("Room: %d\n", status.room);
        #endif
        switch ( *input )
        {
            case 'Q':
            case 'q':
                //printf("quit called\n");
                control('q');
                break;
            case 'N':
            case 'n':
                gotoroom(0);
                break;
            case 'S':
            case 's':
                gotoroom(1);
                break;
            case 'E':
            case 'e':
                gotoroom(2);
                break;
            case 'W':
            case 'w':
                gotoroom(3);
                break;
            case 'T':
            case 't':        
                strncpy(buffer, input, 4);
                //printf("\ntake%d", strcmp(buffer, "take"));
                if ( strcmp(buffer, "take") == 0 ) { take(input); free(buffer); break; };
            default :
                printf("?!\n");
                break;
        }
    }
    
    
    void take(const char *input)
    {
        char *buffer = malloc(5); buffcheck(buffer);
        strncat(buffer, (input + 5), 5); /* Append input onto buffer, skipped 5 characters */
        printf("Item Requested:%s\n", buffer);
        free(buffer);
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. C++ Beginner
    By Ronzel in forum C++ Programming
    Replies: 2
    Last Post: 06-04-2009, 03:06 AM
  2. Beginner-need help
    By new-b in forum C Programming
    Replies: 10
    Last Post: 06-04-2009, 12:36 AM
  3. Help a Beginner!
    By Mad Dawg in forum C++ Programming
    Replies: 6
    Last Post: 09-21-2005, 06:18 PM
  4. Windows programming for beginner (Absolute beginner)
    By WDT in forum Windows Programming
    Replies: 4
    Last Post: 01-06-2004, 11:21 AM
  5. C++ Beginner Needs Help!!!
    By DeanDemon in forum C++ Programming
    Replies: 2
    Last Post: 11-29-2002, 09:23 AM

Tags for this Thread