Re: patch.tgz

Hello nobu,

Great suggestion. This lead to a few compile errors on the patch that I
proposed but they were minimal. Basically VALUEFUNC needs to be removed from
the functions passed to rb_exec_recursive. I think a similar system could be
used for rb_define_method as it would standardize the interface. The only
downside is that no longer specialized functions (with e.g. only 2
parameterscan be made if for instance your method takes 1 argument
(+self).

Regards,
Christophe

On 10/5/05, [e-mail] <[e-mail]> wrote:
>
> Hi,
>
> At Wed, 5 Oct 2005 02:26:33 +0900,
> Christophe Poucet wrote in [ruby-core:06143]:
> > [0 patch.tgz <application/x-gzip (base64)>]
>
> I don't think casting function pointers is safe. This patch
> would fix some sort of warnings. And I found qsort in util.h
> is incompatible to ANSI C.
>
>
> Index: array.c
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/array.c,v
> retrieving revision 1.180
> diff -U2 -p -r1.180 array.c
> --- array.c 24 Sep 2005 00:17:40 -0000 1.180
> +++ array.c 4 Oct 2005 22:55:11 -0000
> @@ -1246,6 +1246,7 @@ extern VALUE rb_output_fs;
>
> static VALUE
> -recursive_join(VALUE ary, VALUE *arg, int recur)
> +recursive_join(VALUE ary, VALUE argp, int recur)
> {
> + VALUE *arg = (VALUE *)argp;
> if (recur) {
> return rb_str_new2("[...]");
> @@ -1474,19 +1475,20 @@ ary_sort_check(struct ary_sort_data *dat
>
> static int
> -sort_1(VALUE *a, VALUE *b, struct ary_sort_data *data)
> +sort_1(const void *ap, const void *bp, void *data)
> {
> - VALUE retval = rb_yield_values(2, *a, *b);
> + VALUE a = *(const VALUE *)ap, b = *(const VALUE *)bp;
> + VALUE retval = rb_yield_values(2, a, b);
> int n;
>
> - n = rb_cmpint(retval, *a, *b);
> - ary_sort_check(data);
> + n = rb_cmpint(retval, a, b);
> + ary_sort_check((struct ary_sort_data *)data);
> return n;
> }
>
> static int
> -sort_2(VALUE *ap, VALUE *bp, struct ary_sort_data *data)
> +sort_2(const void *ap, const void *bp, void *data)
> {
> VALUE retval;
> - VALUE a = *ap, b = *bp;
> + VALUE a = *(const VALUE *)ap, b = *(const VALUE *)bp;
> int n;
>
> @@ -1502,5 +1504,5 @@ sort_2(VALUE *ap, VALUE *bp, struct ary_
> retval = rb_funcall(a, id_cmp, 1, b);
> n = rb_cmpint(retval, a, b);
> - ary_sort_check(data);
> + ary_sort_check((struct ary_sort_data *)data);
>
> return n;
> @@ -1514,6 +1516,6 @@ sort_internal(VALUE ary)
> data.ary = ary;
> data.ptr = RARRAY(ary)->ptr; data.len = RARRAY(ary)->len;
> - qsort(RARRAY(ary)->ptr, RARRAY(ary)->len, sizeof(VALUE),
> - rb_block_given_p()?sort_1:sort_2, &data);
> + ruby_qsort(RARRAY(ary)->ptr, RARRAY(ary)->len, sizeof(VALUE),
> + rb_block_given_p()?sort_1:sort_2, &data);
> return ary;
> }
> Index: enum.c
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/enum.c,v
> retrieving revision 1.63
> diff -U2 -p -r1.63 enum.c
> --- enum.c 12 Sep 2005 10:44:19 -0000 1.63
> +++ enum.c 4 Oct 2005 22:57:15 -0000
> @@ -434,8 +434,8 @@ sort_by_i(VALUE i, VALUE ary)
>
> static int
> -sort_by_cmp(NODE **aa, NODE **bb)
> +sort_by_cmp(const void *ap, const void *bp, void *data)
> {
> - VALUE a = aa[0]->u1.value;
> - VALUE b = bb[0]->u1.value;
> + VALUE a = (*(NODE *const *)ap)->u1.value;
> + VALUE b = (*(NODE *const *)bp)->u1.value;
>
> return rb_cmpint(rb_funcall(a, id_cmp, 1, b), a, b);
> @@ -528,5 +528,5 @@ enum_sort_by(VALUE obj)
> rb_iterate(rb_each, obj, sort_by_i, ary);
> if (RARRAY(ary)->len > 1) {
> - qsort(RARRAY(ary)->ptr, RARRAY(ary)->len, sizeof(VALUE), sort_by_cmp,
> 0);
> + ruby_qsort(RARRAY(ary)->ptr, RARRAY(ary)->len, sizeof(VALUE),
> sort_by_cmp, 0);
> }
> if (RBASIC(ary)->klass) {
> Index: eval.c
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/eval.c,v
> retrieving revision 1.834
> diff -U2 -p -r1.834 eval.c
> --- eval.c 28 Sep 2005 22:22:31 -0000 1.834
> +++ eval.c 4 Oct 2005 23:06:41 -0000
> @@ -12946,7 +12946,5 @@ recursive_pop(void)
>
> VALUE
> -rb_exec_recursive(VALUE (*func) (/* ??? */), VALUE obj, VALUE arg)
> - /* VALUE obj, VALUE arg, int flag */
> -
> +rb_exec_recursive(VALUE (*func)(VALUE, VALUE, int), VALUE obj, VALUE arg)
> {
> if (recursive_check(obj)) {
> Index: file.c
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/file.c,v
> retrieving revision 1.212
> diff -U2 -p -r1.212 file.c
> --- file.c 28 Sep 2005 14:40:25 -0000 1.212
> +++ file.c 4 Oct 2005 22:59:26 -0000
> @@ -2696,6 +2696,7 @@ static VALUE rb_file_join(VALUE ary, VAL
>
> static VALUE
> -file_inspect_join(VALUE ary, VALUE *arg, int recur)
> +file_inspect_join(VALUE ary, VALUE argp, int recur)
> {
> + VALUE *arg = (VALUE *)arg;
> if (recur) return rb_str_new2("[...]");
> return rb_file_join(arg[0], arg[1]);
> Index: intern.h
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/intern.h,v
> retrieving revision 1.184
> diff -U2 -p -r1.184 intern.h
> --- intern.h 1 Oct 2005 04:11:43 -0000 1.184
> +++ intern.h 4 Oct 2005 23:04:05 -0000
> @@ -282,5 +282,5 @@ VALUE rb_thread_local_aref(VALUE, ID);
> VALUE rb_thread_local_aset(VALUE, ID, VALUE);
> void rb_thread_atfork(void);
> -VALUE rb_exec_recursive(VALUE(*)(ANYARGS),VALUE,VALUE);
> +VALUE rb_exec_recursive(VALUE(*)(VALUE, VALUE, int),VALUE,VALUE);
> /* file.c */
> int eaccess(const char*, int);
> Index: util.c
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/util.c,v
> retrieving revision 1.47
> diff -U2 -p -r1.47 util.c
> --- util.c 14 Sep 2005 06:32:32 -0000 1.47
> +++ util.c 4 Oct 2005 22:50:40 -0000
> @@ -471,10 +471,7 @@ typedef struct { char *LL, *RR; } stack_
> ((*cmp)(b,c,d)>0 ? b : ((*cmp)(a,c,d)<0 ? a : c)))
>
> -void ruby_qsort (base, nel, size, cmp, d)
> - void* base;
> - const int nel;
> - const int size;
> - int (*cmp)();
> - void *d;
> +void
> +ruby_qsort(void* base, const int nel, const int size,
> + int (*cmp)(const void*, const void*, void*), void *d)
> {
> register char *l, *r, *m; /* l,r:left,right group m:median point */
> Index: util.h
> ===================================================================
> RCS file: /cvs/ruby/src/ruby/util.h,v
> retrieving revision 1.17
> diff -U2 -p -r1.17 util.h
> --- util.h 14 Sep 2005 06:32:32 -0000 1.17
> +++ util.h 4 Oct 2005 22:48:28 -0000
> @@ -44,6 +44,5 @@ void ruby_add_suffix(VALUE str, char *su
> #endif
>
> -void ruby_qsort(void*, const int, const int, int (*)(), void*);
> -#define qsort(b,n,s,c,d) ruby_qsort(b,n,s,c,d)
> +void ruby_qsort(void*, const int, const int, int (*)(const void*,const
> void*,void*), void*);
>
> void ruby_setenv(const char*, const char*);
>
>
> --
> Nobu Nakada
>
>