Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation error in Disk API #5

Closed
ghost opened this issue Jun 27, 2016 · 5 comments
Closed

Documentation error in Disk API #5

ghost opened this issue Jun 27, 2016 · 5 comments
Assignees

Comments

@ghost
Copy link

ghost commented Jun 27, 2016

I have found another error in the MikeOS documentation.

All disk functions use the carry flag to report a disk error, except os_get_file_list which returns a blank string. However, the MikeOS documentation fails to mention that os_create_file and os_remove_file use the carry flag to indicate an error.

I checked your implementation and found your 'load_file', 'create_file' and 'remove_file' functions do not handle the carry flag. Additionally, in 'get_file_size' you return zero as the file size if a disk error occurs, which may be misleading.

@I8087
Copy link
Owner

I8087 commented Jun 27, 2016

Thanks for bringing this to my attention! I'll go through each of those functions later tonight or tomorrow to figure out what needs to be done. Unfortunately, mlib.h shows that get_file_size and load_file already return a value, so I think it'd be more of the programmer's responsibility to take care of it, rather than the libraries. For example, the code below should be sufficient enough in most cases.

#include "mlib.h"

int main(int argc, char** argv) {
    char *file = "example.txt";
    if(!file_exists(file)) {
        size = get_file_size(file);
        if (size < 28671) {
            /* Load 4 KiB away from this program. */
            load_file(file, 28672); /* This should probably return a pointer. */
                                    /* Example:  int *p = load_file(file, 28672); */
        } else {
            print_string("The File is too large to load!\r\n");
        }
    } else {
        print_string("Error loading the file!\r\n");
    }
}

This should catch most problems caused by disk I/O.

@I8087
Copy link
Owner

I8087 commented Jun 28, 2016

Actually, it might be better to have a global variable IO_FLAGS and every time a hardware I/O is preformed, the value of IO_FLAGS is updated with the current system flags. That way, you can be certain there's was a success/failure.

@ghost
Copy link
Author

ghost commented Jun 28, 2016

You could also return multiple values from the function by having an extra parameter that is an int* type. That is how I have currently implemented it. However, that is entirely up to individual style.

If you want to know the memory location after the end of the program, the Smaller C linker has a macro 'stop_alldata', that should expand to the location in memory after the program. Like so:

extern unsigned int _stop_all_data__;
#define ramstart &_stop_all_data__
load_file(file, ramstart);

Hopefully the C compiler will not mangle the constant name.

I am a little concerned about the return value of MikeOS's os_get_file_size, in that it does not account for "slack space". I will raise this issue on the MikeOS forum, in the meantime you should round up to the nearest 512 bytes.

@I8087
Copy link
Owner

I8087 commented Jun 28, 2016

Thanks for all the info, I think the easiest way would be to implement a global variable kinda like C's errno, but like you said it's all up to individual style. Also, thanks for those two last tid bits. It's nice to know exactly where the program ends and silly me, I forget MikeOS loads the entire end sector, even if it's not completely used.

@I8087 I8087 self-assigned this Jul 1, 2016
@I8087
Copy link
Owner

I8087 commented Jul 1, 2016

Alright, ioerr will be used. The list below will be used to help me keep track of its implementation.

  • Disk API
  • Serial API

@I8087 I8087 closed this as completed in 933f321 Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant