0

I am writing a shell not so fancy but want to implement some functions. Here is my current code.

int readCommand(char * buffer, char * commandInput) {
    buf_chars = 0;
    char c;
    while((*commandInput != '\n') && (buf_chars < LINE_LEN)) {
        buffer[buf_chars++] = *commandInput;
        scanf("%c", &c);
        if(c==0x0C)
        {
            buffer[buf_chars] = '\0';
            printf("\033[2J\033[1;1H");//clear screen
            printf("%s",buffer);
            continue;
        }
        *commandInput = c;
    }
    buffer[buf_chars] = '\0';
    return 0;
}

It should clear the screen and than print the current command being typed. I dont know what i am doing wrong.

3
  • when compiling, always enable all the warnings, then fix those warnings. 1) variable buf_chars not defined. 2) in the while() statement, the expression: *commandInput != '\n' is comparing an array of char to a single char. perhaps you meant: (commandInput[0] != '\n') 3) LINE_LEN not defined in the posted code. 4) this statement: buffer[buf_chars++] = *commandInput; will ALWAYS copy the FIRST char in the array of char pointed to by commandInput to which char is being selected in buffer[] via the buf_chars variable. 5) the 0x0C is a formfeed. Is that what you want? Commented May 21, 2016 at 22:55
  • one way to get out of the while() loop is if buf_chars is equal to LINE_LEN. However, if buffer only has room for LINE_LEN characters, then the line, after the end of the while() loop: buffer[buf_chars] = '\0'; will be writing past the end of the char array pointed to by buffer resulting in undefined behaviour and can lead to a seg fault event. Commented May 21, 2016 at 22:59
  • when posting code, always include the relevant #include statements, so we do not have to guess if your actual code includes the appropriate header files. Commented May 21, 2016 at 23:00

2 Answers 2

1

There are several things wrong with this.

  1. Why in the world would you want to clear the screen in response to receiving a form feed character? That seems incredibly random.

  2. If you indeed do want to clear the screen in response to that character, then presumably its job is then done. Why would you copy it to the buffer?

  3. Turning more directly to your question, you appear to be trying to output an ANSI escape sequence. Not all terminals recognize those. Moreover,

  4. stdout is likely to be line buffered, and the data you are writing are certain to not contain a newline. As a result, the buffer is unlikely to be flushed to the output device at that point. You could fix that by fflush()ing stdout after the printf() calls.

  5. However, your second printf() is faulty, because you have not ensured that the data you have copied into the buffer are null-terminated, and the printf() format does not contain a field width that would make it stop before the end of the buffer.

Sign up to request clarification or add additional context in comments.

Comments

1

Reading input for your shell, is probably better done with fgets (or getline). Then all you need do is (1) validate the pointer returned by fgets (to check EOF), (2) check the last character in the buffer to determine if it is '\n' (otherwise a short read occurred, and more characters remain to be read) and (3) remove the '\n' from the end of the buffer before using the buffer as the command.

You can accomplish all three in the same length of code you are presently using. For example you could do something like the following:

char *readcmd (char *buf, size_t max, FILE *fp)
{
    if (!buf)       /* validate buf not NULL */
        return NULL;

    char *p = NULL

    /* read input from fp, return NULL on EOF */
    if (!(p = fgets (buf, max, fp)))
        return NULL;

    /* find '\n' or nul-terminating char */
    for (; *p && *p != '\n'; p++) {}

    /* if not '\n' - short read occurred, read/discard remaining */
    if (*p != '\n')
    {
        int c;
        fprintf (stderr, "error: line too long, exceeds %zu chars.\n", max);

        /* read/discard chars reamining in buffer to '\n' */
        while ((c = fgetc (fp)) != '\n' && c != EOF) {}

        return NULL;
    }
    *p =  0;    /* overwrite '\n' with nul-terminating char */

    return buf;
}

(note: you can change how you respond to a short read to meet your own needs. If you dynamically allocate buf, you can realloc and continue reading. You can also look at using getline instead of fgets which will automatically allocate enough memory to read any line no matter how long (to the extent of available memory), but consider the potential problem with getline as well -- it will read any line no matter how long -- which when taking user input may not be what you want)

A short example showing how you might utilize the function to take input for your shell might be:

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

#define MAXC 256

char *readcmd (char *buf, size_t max, FILE *fp);

int main (int argc, char **argv) {

    char buf[MAXC] = "";
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;
    if (!fp) {
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    for (;;) {    /* prompt & read command until 'quit or EOF */
        printf ("> ");

        if (!readcmd (buf, MAXC, fp)) { /* EOF or short read   */
            putchar ('\n');             /* no '\n', so tidy up */
            break;
        }
        if (!strcmp (buf, "quit"))      /* 'quit' entered */
            break;

        /* handle command here */
        printf ("  execute command : '%s'\n", buf);
    }

    if (fp != stdin) fclose (fp); /* close if not reading stdin */

    return 0;
}
/* -- include readcmd definition here -- */

Which prompts the user with > for input, and limits the number of characters in a command to 256 (including the terminating char). The code exits when readcmd encounters an EOF or when too many characters are entered (a short read). It will also exit if the user enters quit.


Handling Key ScanCodes

If you want to hande key scan code such as to detect Control + l, then you must process the integer values that make up the codes. There are a number of ways to do this, but the basic approach is to generate a list of integers that make up the multi-byte scan codes on your system and then check for them in your code. This is done below by filling the 5-int scncode array with up to 5-int of the information read by fgets. You can then check the integer codes against those used by any of the Control, Shift, Alt, etc. code and the letter to determine if a know key combination was pressed.

Below Control + l (ell) provides a single integer scancode combination of 12. Upon receiving Control + l as input alone, a message is generated that ==> [ctrl + l (ell)] pressed..

char *readcmd (char *buf, size_t max, FILE *fp)
{
    if (!buf) return NULL;       /* validate buf not NULL */

    char *p = NULL;
    int scncode[5] = {0};   /* scncode and sz to trap ctrl+l */
    size_t sz = 0;

    /* read input from fp, return NULL on EOF */
    if (!(p = fgets (buf, max, fp)))
        return NULL;

    /* find '\n' or nul-terminating char, set len */
    for (; *p && *p != '\n'; p++) {}
    sz = p - buf;

    /* if not '\n' - short read occurred, read/discard remaining */
    if (*p != '\n')
    {
        int c;
        fprintf (stderr, "error: line too long, exceeds %zu chars.\n", max);

        /* read/discard chars reamining in buffer to '\n' */
        while ((c = fgetc (fp)) != '\n' && c != EOF) {}

        return NULL;
    }
    *p =  0;    /* overwrite '\n' with nul-terminating char */
    memcpy (scncode, buf, sz > 5 ? 5 : sz); /* fill scancode with buffer */

    /* check byte/multibyte char against scancode */
    if (scncode[0] == 12) fprintf (stderr, "==> [ctrl + l (ell)] pressed.\n");

    return buf;
}

Look things over and let me know if you have any questions.

1 Comment

you are not handling ctrl+L character anywhere my original code works except for handling ctrl+L character.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.