Hi, Maxime Devos <maximede...@telenet.be> skribis:
> The only thing missing for me, is a procedure > 'bytevector-slice/read-only' and 'bytevector-slice/write-only', then I > could throw the module implementing the wrapping in Scheme-GNUnet and > the overhead incurred by wrapping away. I did consider the read-only part, but it’s not really implementable currently as SCM_F_BYTEVECTOR_IMMUTABLE flag is even ignored by instructions, and fixing it is far beyond the scope of this patch: https://issues.guix.gnu.org/60779 > On 11-01-2023 16:00, Ludovic Courtès wrote: [...] >> +(use-modules (rnrs bytevectors) >> + (rnrs bytevectors gnu)) > > I thought that R6RS reserved (rnrs ...) to the RnRS process, so I > would think that naming it (rnrs bytevectors gnu) would be > standards-incompliant, though I cannot find in the specification, so > maybe it isn't actually reserved. > > (SRFI looks a bit looser to me, so I find the (srfi ... gnu) > acceptable, but (rnrs ... gnu) looks weird to me, I would propose > (ice-9 bytevector-extensions) or such.). I’ll stick to gnu.scm because that’s the convention we’ve used for extensions so far, and “gnu” is arguably “reserved”. >> +Copyright (C) 1996-1997, 2000-2005, 2009-2023 Free Software Foundation, > > > Where does this year 2022 come from? Does a previous version of this > patch predate the new year? I started working on it in December and my ‘write-file-hooks’ updated it. >> + c_offset = scm_to_size_t (offset); >> + >> + if (SCM_UNBNDP (size)) >> + { >> + if (c_offset < SCM_BYTEVECTOR_LENGTH (bv)) >> + c_size = SCM_BYTEVECTOR_LENGTH (bv) - c_offset; >> + else >> + c_size = 0; > + } >> + else >> + c_size = scm_to_size_t (size); >> + >> + if (c_offset + c_size > SCM_BYTEVECTOR_LENGTH (bv)) >> + scm_out_of_range (FUNC_NAME, offset); > > > If offset=SIZE_MAX-1 and size=1, this will overflow to 0 and hence not > trigger the error reporting. This bounds check needs to be rewritten, > with corresponding additional tests. OK. > IIUC, if you use bytevector-slice iteratively, say: > > (let loop ((bv some-initial-value) > (n large-number)) > (if (> n 0) > (loop (bytevector-slice bv 0 (bytevector-length bv)) > (- n 1)) > bv)) > > you will end up with a bytevector containing a reference to a > bytevector containing a reference to ... containing a reference to the > original reference, in a chain of length ≃ large-number. The ‘parent’ word is here just so the backing memory isn’t reclaimed while the slice is still alive. Whether there’s a long chain of ‘parent’ links or not makes no difference to GC performance nor to memory usage. > Do you know what this 'parent' field even is for? Going by some > comments in 'libguile/bytevectors.c', it is for GC reasons, but going > by the existence of the 'bytevector->pointer' + 'pointer->bytevector' > trick which destroys 'parent' information, it seems unnecessary to me. Like I wrote, it was introduced to keep backing memory alive, generally. With ‘pointer->bytevector’, we want to make sure the original pointer remains live as long as the bytevector is live (pointer objects can have a finalizer, and that finalizer must not run while the bytevector is live). > Nowadays a https://.../ instead of a mail address, is more > conventional and useful, I'd think. Its even used in old files, > e.g. libguile/r6rs-ports.c. Noted. > A test is missing for the case where the size is unaligned instead of > the offset. Noted. I’ll come up with a second version taking this into account. Thanks! Ludo’.