[breaking change] 70 shared resources cc65 api #71
Conversation
|
I think |
| extern void shres_trap(void); | ||
|
|
||
| /// Registers used for communicating with the shared resource trap. | ||
| extern unsigned char shres_regs[5]; |
There was a problem hiding this comment.
Does this need to be a global variable? With llvm-mos, taking it as an argument for shres_trap() is usually better as the compiler has more optimization opportunities, e.g. use registers and reduced memory. I would prefer shres_trap() to use inline asm with llvm-mos, again for possible compiler optimization of arguments and return values (if any). I'm a little rusty but would be happy to look into this after the merge. It would be helpful with a test file in tests/ for that purpose.
| * @param f Pointer to an open shared_resource structure. | ||
| * @param offset Byte offset to seek. | ||
| * @param whence One of SEEK_SET, SEEK_CUR, or SEEK_END. | ||
| * @return 0 on success, 1 if seek was out of bounds or f is NULL. |
| * @param file_handle Pointer to a shared_resource structure to populate. | ||
| * @return 0 on success, 1 if not found or error. | ||
| */ | ||
| char shopen(char* resource_name, unsigned long required_flags, |
There was a problem hiding this comment.
char -> bool by including <stdbool.h> (should be available on both cc65 and clang)
|
closes #70 |
| * @param file_handle Pointer to the shared_resource struct to populate. | ||
| * @return 0 on success, 1 if the resource was not found or an error occurred. | ||
| */ | ||
| char shopen(char* resource_name, unsigned long required_flags, |
|
Ping @gardners |
| * (auto-increments). | ||
| * @param dirent Pointer to a shared_resource struct to populate. | ||
| * @return | ||
| * - 0 on success (matching entry read), |
There was a problem hiding this comment.
Perhaps add something like
#define SHDREAD_OK 0
#define SHDREAD_TRAP_FAIL 1
#define SHDREAD_END 2to the header which would make calling code more expressive.
|
|
||
| #ifndef SHRES_H | ||
| #define SHRES_H | ||
|
|
There was a problem hiding this comment.
Allow use from C++ by encapsulating with:
#ifdef __cplusplus
// Being compiled by a C++ compiler, inhibit name mangling
extern "C" {
#endif
...
#ifdef __cplusplus
} // End of extern "C"
#endif
| #define SHRES_H | ||
|
|
||
| /// Triggers the low-level SYSPART shared resource trap. | ||
| extern void shres_trap(void); |
There was a problem hiding this comment.
Add
#ifdef __clang__
__attribute__((leaf))
#endif
in front of function.
| .global _shres_trap | ||
| .global _shres_regs | ||
|
|
||
| .section .text |
There was a problem hiding this comment.
Change to .section .text.shres_trap,"ax",@progbits
src/shres.c
Outdated
|
|
||
| // Verify magic string in sector 0 | ||
| for (i = 0; magic_string[i]; i++) { | ||
| if (lpeek(0xffd6e00L + i) != magic_string[i]) { |
There was a problem hiding this comment.
I would prefer a named address - this seems to be "SD Card controller sector buffer"(?).
| sdcard_busy_wait(); | ||
|
|
||
| // Select SD card buffer, not FDC buffer | ||
| POKE(0xD689L,PEEK(0xD689L)|0x80); |
There was a problem hiding this comment.
Use 0xD689 without L? The POKE/PEEK macros only handle 16-bit addresses as opposed to lpeek/lpoke
| #endif | ||
| uint8_t | ||
| chdirroot(void); | ||
| chdirroot(uint8_t partition); |
There was a problem hiding this comment.
Update tests/test-fileio.c to reflect this breaking change. CI currently fails.
mlund
changed the title
Checklist
Thanks a lot for your contribution!
Please tick off the following:
make test)clang-format -i <file>