Add snprintf implementation #170
Conversation
icgmilk
commented
Dec 3, 2024
icgmilk
commented
- Add snprintf in c.c
- Add tests for snprintf
| } | ||
| } | ||
| if (bi < n) buffer[bi] = '\0'; | ||
| } |
There was a problem hiding this comment.
When using git for version control, it is recommended to add a newline at the end of files. This is because git compares files line by line, and the end of each line is identified by a newline character.
For more details, refer to the following link:
https://medium.com/@alexey.inkin/how-to-force-newline-at-end-of-files-and-why-you-should-do-it-fdf76d1d090e
|
I noticed the PR includes a merge commit, which doesn't seem necessary. Could you remove it? |
|
Currently, shecc doesn't have any direct users of snprintf(). I'm unsure if adding it without a user is acceptable, so I'll leave that for the maintainer to decide. If this change is welcomed, should we aim to support as many functions from the C library standard as possible? |
lib/c.c
Outdated
| @@ -724,3 +724,68 @@ void free(void *ptr) | |||
| __freelist_head->prev = cur; | |||
| __freelist_head = cur; | |||
| } | |||
|
|
|||
| void snprintf(char *buffer, size_t n, char *str, ...) | |||
There was a problem hiding this comment.
Currently shecc doesn't support unsigned types, size_t is unsigned type. Instead, use int as a workaround at this moment.
| try_output "Hello World 1123" << EOF | ||
| int main() { | ||
| char buffer[50]; | ||
| snprintf(buffer, sizeof(buffer), "Hello %s %d", "World", 1123); | ||
| printf("%s", buffer); | ||
| return 0; | ||
| } | ||
| EOF | ||
|
|
||
| try_output "Number: -37" << EOF | ||
| int main() { | ||
| char buffer[20]; | ||
| snprintf(buffer, sizeof(buffer), "Number: %d", -37); | ||
| printf("%s", buffer); | ||
| return 0; | ||
| } | ||
| EOF |
There was a problem hiding this comment.
I don't to see any edge case here, please add an edge case that covers when given formatted string's length exceeds n.
lib/c.c
Outdated
| int si = 0, bi = 0, pi = 0; | ||
|
|
||
| while (str[si]) { | ||
| if (bi >= n-1) { |
There was a problem hiding this comment.
Please keep the coding convention.
| int *var_args = &str + 4; | ||
| int si = 0, bi = 0, pi = 0; | ||
|
|
||
| while (str[si]) { |
There was a problem hiding this comment.
Could we reuse the code in sprintf() ?
| return 0; | ||
| } | ||
| EOF | ||
|
|
There was a problem hiding this comment.
Could you provide more tests such as string truncation?
|
Additionally, I failed to see why this build keeps failed on stage1. I've used gdb to debug the cause, but I've found that by applying the following diff in diff --git a/lib/c.c b/lib/c.c
index c76bde9..412c485 100644
--- a/lib/c.c
+++ b/lib/c.c
@@ -725,7 +725,7 @@ void free(void *ptr)
__freelist_head = cur;
}
-void snprintf(char *buffer, size_t n, char *str, ...)
+void snprintf(char *buffer, int n, char *str, ...)
{
int *var_args = &str + 4;
int si = 0, bi = 0, pi = 0;
@@ -787,5 +787,5 @@ void snprintf(char *buffer, size_t n, char *str, ...)
si++;
}
}
- if (bi < n) buffer[bi] = '\0';
+ if (bi < n) buffer[bi] = 0;
}
\ No newline at end of file
The program would failed at |
|
@vacantron do you have any suspicion for the above ssa error? |
I haven't checked it yet. But I think there is an array index out of bound somewhere. |
Replaced 'size_t' type with 'int' in the function due to shecc limitations that do not support the 'unsigned' type.
Refactor function 'snprintf(size_t n)' to replace 'size_t' with 'int'
|
FYI: tinyprintf implements printf and sprintf for small embedded systems. |
