Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Commit 8143ff4

Browse files
author
Erik Hollensbe
committed
Fix an issue where the args list would get corrupted if Procs were used.
The issue seems to center around the union types that mruby uses. Examining the anomaly I noticed the proc values were getting truncated -- they are a part of a union and maybe mruby is doing something weird here. Either way, copying the struct (leaving the data unaltered) as opposed to passing the pointer, seems to preserve the width information which is what was causign the issue. I strongly believe this fixes two bugs; one in argument handling of proc and another where certain values would get corrupted and could not be extracted as string. The rest of this is cleanup and rearch for simpler code. Signed-off-by: Erik Hollensbe <[email protected]>
1 parent 219b7d9 commit 8143ff4

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

args.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package mruby
22

3-
import (
4-
"sync"
5-
)
3+
import "sync"
64

75
// #include "gomruby.h"
86
import "C"
@@ -45,10 +43,10 @@ func ArgsOpt(n int) ArgSpec {
4543

4644
// The global accumulator when Mrb.GetArgs is called. There is a
4745
// global lock around this so that the access to it is safe.
48-
var getArgAccumulator []*C.mrb_value
49-
var getArgLock sync.Mutex
46+
var getArgAccumulator []C.mrb_value
47+
var getArgLock = new(sync.Mutex)
5048

5149
//export goGetArgAppend
52-
func goGetArgAppend(v *C.mrb_value) {
50+
func goGetArgAppend(v C.mrb_value) {
5351
getArgAccumulator = append(getArgAccumulator, v)
5452
}

gomruby.h

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#ifndef _GOMRUBY_H_INCLUDED
88
#define _GOMRUBY_H_INCLUDED
99

10+
#include <errno.h>
1011
#include <mruby.h>
1112
#include <mruby/array.h>
1213
#include <mruby/class.h>
@@ -21,6 +22,13 @@
2122
#include <mruby/value.h>
2223
#include <mruby/variable.h>
2324

25+
// (erikh) this can be set in mruby/mrbconfig.h so we can default it here.
26+
// XXX I don't know how this actually plays out when the config is modified.
27+
// I'm taking a WAG here. Either way, the default is 16 in vm.c.
28+
#ifndef MRB_FUNCALL_ARGC_MAX
29+
#define MRB_FUNCALL_ARGC_MAX 16
30+
#endif // MRB_FUNCALL_ARGC_MAX
31+
2432
//-------------------------------------------------------------------
2533
// Helpers to deal with calling back into Go.
2634
//-------------------------------------------------------------------
@@ -90,24 +98,32 @@ static mrb_value _go_mrb_yield_argv(mrb_state *mrb, mrb_value b, mrb_int argc, c
9098
// Helpers to deal with getting arguments
9199
//-------------------------------------------------------------------
92100
// This is declard in args.go
93-
extern void goGetArgAppend(mrb_value*);
101+
extern void goGetArgAppend(mrb_value);
94102

95103
// This gets all arguments given to a function call and adds them to
96104
// the accumulator in Go.
97105
static inline int _go_mrb_get_args_all(mrb_state *s) {
98106
mrb_value *argv;
99107
mrb_value block;
108+
mrb_bool append;
100109
int argc, i, count;
101110

102-
count = mrb_get_args(s, "*&", &argv, &argc, &block);
111+
count = mrb_get_args(s, "*&?", &argv, &argc, &block, &append);
112+
if (count < 0 || errno != 0) {
113+
return count;
114+
}
115+
103116
for (i = 0; i < argc; i++) {
104-
goGetArgAppend(&argv[i]);
117+
goGetArgAppend(argv[i]);
105118
}
106119

107-
if (!mrb_nil_p(block)) {
108-
goGetArgAppend(&block);
120+
if (append == FALSE || mrb_type(block) == MRB_TT_FALSE) {
121+
return count;
109122
}
110123

124+
count++;
125+
goGetArgAppend(block);
126+
111127
return count;
112128
}
113129

@@ -198,6 +214,10 @@ static inline void _go_enable_gc(mrb_state *m) {
198214
_go_set_gc(m, 0);
199215
}
200216

217+
static inline int _go_get_max_funcall_args() {
218+
return MRB_FUNCALL_ARGC_MAX;
219+
}
220+
201221
// this function returns 1 if the value is dead, aka reaped or otherwise
202222
// terminated by the GC.
203223
static inline int _go_isdead(mrb_state *m, mrb_value o) {

mruby.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -147,23 +147,18 @@ func (m *Mrb) GetArgs() []*MrbValue {
147147
getArgLock.Lock()
148148
defer getArgLock.Unlock()
149149

150-
// If we haven't initialized the accumulator yet, do it. We then
151-
// keep this slice cached around forever.
152-
if getArgAccumulator == nil {
153-
getArgAccumulator = make([]*C.mrb_value, 0, 5)
154-
}
150+
// Clear reset the accumulator to zero length
151+
getArgAccumulator = make([]C.mrb_value, 0, C._go_get_max_funcall_args())
155152

156153
// Get all the arguments and put it into our accumulator
157-
C._go_mrb_get_args_all(m.state)
154+
count := C._go_mrb_get_args_all(m.state)
158155

159156
// Convert those all to values
160-
values := make([]*MrbValue, len(getArgAccumulator))
161-
for i, v := range getArgAccumulator {
162-
values[i] = newValue(m.state, *v)
163-
}
157+
values := make([]*MrbValue, count)
164158

165-
// Clear reset the accumulator to zero length
166-
getArgAccumulator = make([]*C.mrb_value, 0, 5)
159+
for i := 0; i < int(count); i++ {
160+
values[i] = newValue(m.state, getArgAccumulator[i])
161+
}
167162

168163
return values
169164
}

0 commit comments

Comments
 (0)