Skip to content

Use ptrdiff_t instead of ssize_t - #10

Closed
jibsen wants to merge 1 commit into
google:masterfrom
jibsen:squash-msvc-support
Closed

Use ptrdiff_t instead of ssize_t#10
jibsen wants to merge 1 commit into
google:masterfrom
jibsen:squash-msvc-support

Conversation

@jibsen

Copy link
Copy Markdown

The compression code seems pretty standard compliant, except for the two places ssize_t (POSIX) is used.

Using ptrdiff_t should allow the snappy plugin for squash to compile with MSVC. See quixdb/squash#145.

@jibsen
jibsenforce-pushed the squash-msvc-support branch from 9f35817 to 56be909CompareJanuary 9, 2016 10:18
@benvanik

Copy link
Copy Markdown

Poke poke owners - can this be merged?

@sesse

Copy link
Copy Markdown
Contributor

Hi,

In general, if your platform doesn't support a given type, that's the role of config.h to fix, not the code itself. MSVC has its own nonstandard type SSIZE_T which works well for this purpose; just add a typedef to your config.h.

@sessesesse closed this Jan 19, 2016
@jibsen

Copy link
Copy Markdown
Author

Personally, I think using a type not in the C standard when there is a suitable type present, is needlessly limiting the portability of code. But at least there is a config.h to work around it.

Also, strictly speaking ssize_t is only guaranteed to support the range [-1, SSIZE_MAX] (it is not a signed size_t, but a size_t with one error value), so the use in IncrementalCopyFastPath could potentially be implementation defined.

ncopa pushed a commit to ncopa/snappy that referenced this pull request Feb 22, 2017
The commit 8bfb028 (Improve zippy decompression speed) introduce a crash
when snappy is compiled with _FORTIFY_SOURCE with musl libc. Backtrace
reveals that it it comes from using memcpy with overlap, which is
undefined behavior.
We avoid the undefined behavior due to memcpy with overlap by using a
temporary uint64_t.
Bactrace from core dump created with `make check`:
````
(gdb) bt
#0 memcpy (__n=8, __os=0xb38c8367eea, __od=0xb38c8367eeb)
at /usr/include/fortify/string.h:48
google#1 snappy::(anonymous namespace)::UnalignedCopy64 (src=0xb38c8367eea,
dst=0xb38c8367eeb) at snappy.cc:92
google#2 0x00006fb4c7c31717 in snappy::(anonymous namespace)::IncrementalCopy
(
buf_limit=0xb38c8380ee0 "", op_limit=<optimized out>, op=<optimized
out>,
src=0xb38c8367eea " .\001") at snappy.cc:178
google#3 snappy::SnappyArrayWriter::AppendFromSelf (len=<optimized out>,
offset=<optimized out>, this=<synthetic pointer>) at snappy.cc:1131
google#4
snappy::SnappyDecompressor::DecompressAllTags<snappy::SnappyArrayWriter>
(
writer=<synthetic pointer>, this=0x7f1d26737050) at snappy.cc:715
google#5 snappy::InternalUncompressAllTags<snappy::SnappyArrayWriter> (
uncompressed_len=<optimized out>, writer=<synthetic pointer>,
decompressor=0x7f1d26737050) at snappy.cc:799
google#6 snappy::InternalUncompress<snappy::SnappyArrayWriter> (
writer=<synthetic pointer>, r=0x7f1d26737000) at snappy.cc:789
google#7 snappy::RawUncompress (compressed=compressed@entry=0x7f1d267370c0,
uncompressed=0xb38c8367ee0 " content: .\001") at snappy.cc:1149
google#8 0x00006fb4c7c3194d in snappy::RawUncompress (compressed=<optimized
out>,
n=<optimized out>, uncompressed=<optimized out>) at snappy.cc:1144
google#9 0x00000b38c5c6261a in snappy::BM_UFlat (iters=99, arg=<optimized
out>)
at snappy_unittest.cc:1371
google#10 0x00000b38c5c69846 in snappy::Benchmark::Run (this=0xb38c8323c40)
at snappy-test.cc:192
google#11 0x00000b38c5c5f3fd in RunSpecifiedBenchmarks () at snappy-test.h:485
google#12 main (argc=1, argv=0x7f1d267374b8) at snappy_unittest.cc:1515
````
Signed-off-by: Natanael Copa <natanael.copa@docker.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jibsen@benvanik@sesse