Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ptrdiff_t instead of ssize_t #10

Closed
wants to merge 1 commit into from

Conversation

jibsen
Copy link

@jibsen jibsen commented Oct 24, 2015

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.

@benvanik
Copy link

Poke poke owners - can this be merged?

@sesse
Copy link
Contributor

sesse commented Jan 19, 2016

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.

@sesse sesse closed this Jan 19, 2016
@jibsen
Copy link
Author

jibsen commented Jan 19, 2016

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 free to 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