Hi Watson,

Sorry for late response. I think your benchmark code contains too much side effects (such as the allocation of 'a' string and hash internals during rehash). If you just benchmark h[42]=42 you should see a difference.

Anyways, I corrected the change to use the mask flag in r5187.

Laurent

On Jan 21, 2011, at 3:09 PM, Watson wrote:

Hi Laurent,

There is only the extremely slight difference.
I think that I can ignore this.
How do you think?

I attached a patch and benchmark script.


** Result of r5185
$ macruby bm_hash.rb
Rehearsal ------------------------------------
 13.640000   0.210000  13.850000 (  9.114591)
-------------------------- total: 13.850000sec

      user     system      total        real
 13.720000   0.240000  13.960000 (  9.172110)


** Result of r5186
$ macruby bm_hash.rb
Rehearsal ------------------------------------
 13.660000   0.210000  13.870000 (  9.239979)
-------------------------- total: 13.870000sec

      user     system      total        real
 13.850000   0.220000  14.070000 (  9.412910)


** Result of appling attached patch
$ macruby bm_hash.rb
Rehearsal ------------------------------------
 13.660000   0.200000  13.860000 (  9.171733)
-------------------------- total: 13.860000sec

      user     system      total        real
 13.790000   0.220000  14.010000 (  9.271439)



2011/1/22 Laurent Sansonetti <lsansonetti@apple.com>:
Hi Watson,
This isn't good, rhash_modify() must be very fast, so calling OBJ_FROZEN and
OBJ_UNTRUSTED is not good there.
Can we look up the mask flag as before?
Laurent
On Jan 21, 2011, at 7:51 AM, source_changes@macosforge.org wrote:

Revision 5186 Author watson1978@gmail.com Date 2011-01-21 07:51:01 -0800
(Fri, 21 Jan 2011)

Log Message

More method of Hash will throw a SecurityError when $SAFE is 4.

Test Script:
{{{
h = {}
$SAFE = 4
h['a'] = 1.0
}}}

Modified Paths

MacRuby/trunk/hash.h

Diff

Modified: MacRuby/trunk/hash.h (5185 => 5186)

--- MacRuby/trunk/hash.h 2011-01-21 02:20:08 UTC (rev 5185)
+++ MacRuby/trunk/hash.h 2011-01-21 15:51:01 UTC (rev 5186)
@@ -41,14 +41,11 @@
static inline void
rhash_modify(VALUE hash)
{
-    const long mask = RBASIC(hash)->flags;
-    if ((mask & FL_FREEZE) == FL_FREEZE) {
- rb_raise(rb_eRuntimeError, "can't modify frozen/immutable hash");
+    if (OBJ_FROZEN(hash)) {
+ rb_error_frozen("hash");
    }
-    if ((mask & FL_TAINT) == FL_TAINT) {
- if (rb_safe_level() >= 4) {
-    rb_raise(rb_eSecurityError, "Insecure: can't modify hash");
- }
+    if (!OBJ_UNTRUSTED(hash) && rb_safe_level() >=  4) {
+ rb_raise(rb_eSecurityError, "Insecure: can't modify hash");
    }
}


_______________________________________________
macruby-changes mailing list
macruby-changes@lists.macosforge.org
http://lists.macosforge.org/mailman/listinfo.cgi/macruby-changes


_______________________________________________
MacRuby-devel mailing list
MacRuby-devel@lists.macosforge.org
http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel


<modify_check_patch.txt><bm_hash.rb>_______________________________________________
MacRuby-devel mailing list
MacRuby-devel@lists.macosforge.org
http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel