[olang/patches/.build.yml] build success

[olang/patches/.build.yml] build success

[builds.sr.ht]

olang/patches/.build.yml: SUCCESS in 37s

[cli: replace memory allocation malloc -> arena][0] from [Johnny Richard][1]

[0]: https://lists.sr.ht/~johnnyrichard/olang-devel/patches/49749
[1]: mailto:johnny@johnnyrichard.com

✓ #1155615 SUCCESS olang/patches/.build.yml https://builds.sr.ht/~johnnyrichard/job/1155615

[PATCH olang] cli: replace memory allocation malloc -> arena

[Johnny Richard]

This code assumes that 1MB will be enough space to accommodate the
entire source code and the AST.  If the source code is greater than half
of the arena capacity, the program will crash.

Signed-off-by: Johnny Richard <johnny@johnnyrichard.com>
---
 src/0c.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/0c.c b/src/0c.c
index 978b770..f0ac974 100644
--- a/src/0c.c
+++ b/src/0c.c
@@ -14,15 +14,19 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
+#include <assert.h>
 #include <errno.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+#include "arena.h"
 #include "lexer.h"
 #include "string_view.h"
 
+#define ARENA_CAPACITY (1024 * 1024)
+
 typedef struct cli_args
 {
     int argc;
@@ -45,7 +49,7 @@ static void
 print_token(char *file_path, token_t *token);
 
 string_view_t
-read_entire_file(char *file_path);
+read_entire_file(char *file_path, arena_t *arena);
 
 int
 main(int argc, char **argv)
@@ -73,7 +77,8 @@ main(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
-    string_view_t file_content = read_entire_file(opts.file_path);
+    arena_t arena = arena_new(ARENA_CAPACITY);
+    string_view_t file_content = read_entire_file(opts.file_path, &arena);
 
     lexer_t lexer = { 0 };
     lexer_init(&lexer, file_content);
@@ -107,7 +112,7 @@ print_usage(FILE *stream, char *prog)
 }
 
 string_view_t
-read_entire_file(char *file_path)
+read_entire_file(char *file_path, arena_t *arena)
 {
     FILE *stream = fopen(file_path, "rb");
 
@@ -122,7 +127,9 @@ read_entire_file(char *file_path)
     file_content.size = ftell(stream);
     fseek(stream, 0, SEEK_SET);
 
-    file_content.chars = (char *)malloc(file_content.size);
+    assert(file_content.size * 2 < ARENA_CAPACITY);
+
+    file_content.chars = (char *)arena_alloc(arena, (size_t)file_content.size);
 
     if (file_content.chars == NULL) {
         fprintf(stderr, "Could not read file %s: %s\n", file_path, strerror(errno));
-- 
2.43.2

Re: [PATCH olang] cli: replace memory allocation malloc -> arena

[Carlos Maniero]

> diff --git a/src/0c.c b/src/0c.c
> index 978b770..f0ac974 100644
> --- a/src/0c.c
> +++ b/src/0c.c
0c.c no longer exists

> +#define ARENA_CAPACITY (1024 * 1024)
Can you add a TODO comment to us to remember to solve this issue in the
feature? We can both make a algorithm to predict how much memory we
gonna need or make arena expandable.

Re: [PATCH olang] cli: replace memory allocation malloc -> arena

[Johnny Richard]

On Tue, Feb 27, 2024 at 12:44:47PM -0300, Carlos Maniero wrote:
> > diff --git a/src/0c.c b/src/0c.c
> > index 978b770..f0ac974 100644
> > --- a/src/0c.c
> > +++ b/src/0c.c
> 0c.c no longer exists
> 
> > +#define ARENA_CAPACITY (1024 * 1024)
> Can you add a TODO comment to us to remember to solve this issue in the
> feature? We can both make a algorithm to predict how much memory we
> gonna need or make arena expandable.

Do you think the following fixup is good enough?

------->8-------
Subject: [PATCH olang] fixup! cli: replace memory allocation malloc -> arena


diff --git a/src/main.c b/src/main.c
index f0ac974..2b2f12a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -25,6 +25,7 @@
 #include "lexer.h"
 #include "string_view.h"
 
+// TODO: find a better solution to define the arena capacity
 #define ARENA_CAPACITY (1024 * 1024)
 
 typedef struct cli_args

Re: [PATCH olang] cli: replace memory allocation malloc -> arena

[Johnny Richard]

On Tue, Feb 27, 2024 at 12:44:47PM -0300, Carlos Maniero wrote:
> > diff --git a/src/0c.c b/src/0c.c
> > index 978b770..f0ac974 100644
> > --- a/src/0c.c
> > +++ b/src/0c.c
> 0c.c no longer exists

missed the reply for this comment...

Sure, since you've applied the rename patch first, I will fixup the file
name as well.  The next patch I will do that.

Re: [PATCH olang] cli: replace memory allocation malloc -> arena

[Carlos Maniero]

> Do you think the following fixup is good enough?
It's perfect! Sorry to hold your patch just for a comment, but since we
don't use any tracking tool for TODOs, I think it is a good idea to add
TODOs comments on the code to keep tracking on them.

Re: [PATCH olang] cli: replace memory allocation malloc -> arena

[Johnny Richard]

On Tue, Feb 27, 2024 at 08:22:32PM -0300, Carlos Maniero wrote:
> > Do you think the following fixup is good enough?
> It's perfect! Sorry to hold your patch just for a comment, but since we
> don't use any tracking tool for TODOs, I think it is a good idea to add
> TODOs comments on the code to keep tracking on them.

No worries.  This patch has been SUPERSEDED by V2[1]

[1]: https://lists.sr.ht/~johnnyrichard/olang-devel/%3C20240228123810.44195-1-johnny%40johnnyrichard.com%3E