Tcl Source Code

View Ticket
Login
Ticket UUID: 2215022
Title: tclBinary.c:TIBC(),T_SBAO(); tclInt.h,tclNamesp.c:TME()
Type: Patch Version: None
Submitter: duoas Created on: 2008-11-02 00:21:36
Subsystem: 12. ByteArray Object Assigned To: patthoyts
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2008-11-08 03:12:04
Resolution: Accepted Closed By: patthoyts
    Closed on: 2008-11-07 20:12:04
Description:
Here are some patches. I don't know exactly which category is appropriate, because it spans a few...

--------------------------------------------------
~/tcl/generic/tclBinary.c
--------------------------------------------------
  TclInitBinaryCmd() had a comment asking for plastic surgery, so I obliged. (Now it is short and beautiful.)

  Tcl_SetByteArrayObj() was also updated to have the 'bytes' == NULL and 'length' < 0 tests in there as described here: http://wiki.tcl.tk/21831
  Tcl_GetByteArrayFromObj() was not modified.

--------------------------------------------------
~/tcl/generic/tclInt.h, ~/tcl/generic/tclNamesp.c
--------------------------------------------------
  TclMakeEnsemble() and EnsembleImplMap were modified to enable the (desired) beautification of TclInitBinaryCmd(). They are fully backwards-compatible with all the other ensemble commands created by the core:
  info   : tclCmdIL.c
  string : tclCmdMZ.c
  dict   : tclDictObj.c
  prefix : tclIndexObj.c
  chan   : tclIOCmd.c
hence their code didn't need to be changed. (But they do need a recompile because struct EnsembleImplMap has new elements in there.)

  The new TclMakeEnsemble() can create sub-command ensembles in addition to global command ensembles. The TclMakeEnsemble() function header and the TclInitBinaryCmd() body document its new powers.

--------------------------------------------------
~/tcl/doc/ByteArrObj.3
--------------------------------------------------
  Updated with two lines indicating that 'bytes' may be NULL even if 'length' > 0 --in which case the newly-allocated byte array contains uninitialized data.


The attached patches were generated with the usual diff command.

Each patch is attached.
Argh, I can only attach one file. Gimme a sec.. I've never diffed more than one file at once before...

Ok, there is now but ONE file. All the CVS junk and directory differences is removed.
  diff -r tcl tcl-modified > foo.diff

Hope this helps.

--Michael
User Comments: patthoyts added on 2008-11-08 03:11:58:
Applied to HEAD now.

patthoyts added on 2008-11-07 07:52:55:
Ah. It turns out the test failure was an artefact from the git repository I use.

patthoyts added on 2008-11-07 07:09:20:

File Added - 300489: 2215022.patch

patthoyts added on 2008-11-07 07:01:36:
I find diff -u is usually a better format than the default as it provides more context.
This patch looks good - it definately improves the binary init function! You should glance through the tcl style guide sometime though. Tcl sources use an indent of 4 and a tab space of 8. When refactoring code try to avoid renaming variables spuriosly. In the ensemble creation reworking you didn't need to redo some of the inner bits and its actually more efficient to stick with Tcl_DString* functions for some of this string manipulation.
I've cleaned up the patch a bit and attached it so you can see what I changed. I'd apply it already except I get a test failure:
binary.test


==== binary-63.2 NaN FAILED
==== Contents of test case:

    binary scan [binary format q -NaN] w w
    format 0x%016lx [expr {$w & 0xfff3ffffffffffff}]

---- Result was:
0xfff3ffffffffffff
---- Result should have been (exact matching):
0xfff0000000000000
==== binary-63.2 FAILED

Once I work out why we should be good to go.

duoas added on 2008-11-02 07:21:36:

File Added - 299806: Duoas-patch.2008-11-01.2014.diff

Attachments: