Iterating over string returns empty in c (os development)

Question

I was making an os, or atleast trying to, but I stumbled upon a problem. While trying to iterate over a string to convert to char to print to screen, the returned char seemed to be empty!(I am actually new to os development); Here is the code snippet:

int offset = 0;

void clear_screen() {
    unsigned char * video = 0xB8000;
    for(int i = 0; i < 2000; i+=2){
        video[i] = ' ';
    }
}

void printc(char c) {
    unsigned char * video = 0xB8000;
    video[offset] = c;
    video[offset+1] = 0x03;
    offset += 2;
}

void print(unsigned char *string) {
    char * sus = '\0';
    uint32 i = 0;
    printc('|');
    sus[0] = 'a';
    printc(sus[0]);  //this prints "a" correctly
    string[i] = 'c';
    while (string[i] != '\0') {
        printc(string[i]);   //this while loop is only called once 
        i++;                 //it prints " " only once and exits
    }
    printc('|');
}

int bootup(void)
{
    clear_screen();

    // printc('h');
    // printc('e');
    // printc('l');                     /* These work */
    // printc('l');
    // printc('o');

    print("hello"); //this doesn't

    return 1;

}

Output that it prints:

|a |

Thanks in advance!!

edit

New print function

void print(unsigned char *string) {
    uint32 i = 0;
    printc('|');
    while (string[i] != '\0') {
        printc('i');  //not printed
        printc(string[i]);
        i++;
    }
    printc('|');
}

still does not work

edit 2 updated the code as per @lundin's advice

int offset = 0;

void clear_screen() {
    unsigned char * video = (unsigned char *)0xB8000;
    for(int i = 0; i < 2000; i+=2){
        video[i] = ' ';
    }
}

void printc(char c) {
    unsigned char * video = (unsigned char *)0xB8000;
    video[offset] = c;
    video[offset+1] = 0x03;
    offset += 2;
}

void print(const char *string) {
    int i = 0;
    printc('|');
    while (string[i] != '\0') {
        printc('i');
        printc(string[i]);
        i++;
    }
    printc('|');
}

int bootup(void)
{
    clear_screen();
    // printc('h');
    // printc('e');
    // printc('l');
    // printc('l');
    // printc('o');
    print("hello");
    return 1;

}

stack:

init_lm:
    mov ax, 0x10
    mov fs, ax          ;other segments are ignored
    mov gs, ax

    mov rbp, 0x90000    ;set up stack
    mov rsp, rbp

    ;Load kernel from disk
    xor ebx, ebx        ;upper 2 bytes above bh in ebx is for cylinder = 0x0
    mov bl, 0x2         ;read from 2nd sectors
    mov bh, 0x0         ;head
    mov ch, 1           ;read 1 sector
    mov rdi, KERNEL_ADDRESS
    call ata_chs_read


    jmp KERNEL_ADDRESS

    jmp $

Answer 1

Your program has undefined behavior since it contains multiple lines that aren't valid C. You will have gotten compiler messages about those lines.

• unsigned char * video = 0xB8000; etc is not valid C, you need an explicit cast. "Pointer from integer/integer from pointer without a cast" issues
• Similarly, char * sus = '\0'; is also not valid C. You are trying to assign a pointer to a single character, which doesn't make sense. String handling beginner FAQ here: Common string handling pitfalls in C programming. It also addresses memory allocation basics.
• sus[0] = 'a'; etc here you have wildly undefined behavior since sus isn't pointing at valid memory.
• In case you are actually trying to access physical memory addresses, this isn't the correct way to do so. You need volatile qualified pointers. See How to access a hardware register from firmware? (In your case it probably isn't a register but everything from that link still applies - how to use hex constants etc.)
• EDIT: void print(unsigned char *string) ... string[i] = 'c'; is also wrong. First of all you are passing a char* which is not necessarily compatible with unsigned char*. Then you shouldn't modify the passed string from inside a function called print, that doesn't make sense. This should have been const char* string to prevent such bugs. As it stands you are passing a string literal to this function and then try to modify it - that is undefined behavior since string literals are read-only.

Assuming gcc or clang, if you wish to block the compiler from generating an executable out of invalid C code, check out What compiler options are recommended for beginners learning C? In your case you also likely need the -ffreestanding option mentioned there.

Answer 2

    char * sus = '\0';

Have not checked more... but this assigns a null pointer to sus, and most probably is not what you want to do.

Answer 3

Before proceeding I would recommend reading the OSDev wiki's page on text-based UIs.

While this may go beyond the scope of the question somewhat, I would strongly recommend that, rather than working with the character/attribute values as unsigned char manually, you might want to declare a struct type for those pairs:

struct TextCell {
    volatile unsigned char ch;
    volatile uint8_t attribute;
};

(You could actually be even more refined about it, by using a bitfield for the individual foreground, background, and decoration components of the attributes, but that's probably getting ahead of things.)

From there you can define the text buffer as a constant pointer:

const struct TextCell* text_buffer = (TextCell *)0xB8000;

You could further define

const uint16_t MAXH = 80, MAXV = 25;
uint16_t currv = 0, currh = 0;
struct TextCell* text_cursor = text_buffer;


void advance_cursor() {
    text_cursor++;
    if (currh < MAXH) {
        currh++;
    }
    else {
        currh = 0;
        if (currv < MAXV) {
            currv++;
        }
        else {
            /* handle scrolling */
        }
    }
}

void gotoxy(uint16_t x, uint16_t y) {
    uint16_t new_pos = x * y;
    if (new_pos > (MAXV * MAXH)) {
        text_cursor = text_buffer + (MAXV * MAXH);
        currh = MAXH;
        currv = MAXV;
    }
    else {
        text_cursor += new_pos;
        currh = x;
        currv = y;
}

Which would lead to the following modifications of your code:

void kprintc(char c, uint8_t attrib) {
    text_cursor->ch = c;
    text_cursor->attribute = attrib;
    advance_cursor();
}

void kprint(const char *string, uint8_t attribs) {
    int i;
    for (i = 0; string[i] != '\0'; i++) {
        kprintc(string[i], attribs);
    }
}

void clear_screen() {
    for(int i = 0; i < (MAXH * MAXV); i++) {
        kprintc(' ', 0);
    }
}

int bootup(void) {
    clear_screen();
    // kprintc('h', 0x03);
    // kprintc('e', 0x03);
    // kprintc('l', 0x03);
    // kprintc('l', 0x03);
    // kprintc('o', 0x03);
    kprint("hello", 0x03);
    return 1;
}

So, why am I suggesting all of this extra stuff? Because it is a lot easier to debug this way, mainly - it divides the concerns up better, and structures the data (or in this case, the video text buffer) more effectively. Also, you'll eventually need to do something like this at some point in the project, so if it helps now, you might as well do it now.

If I am out of line in this, please let me know.