Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Simple Dynamic Strings library for C, compatible with null-terminated strings

github.com

Post reply on HN
Print view View original

41–50 of 85 posts

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#41

Post by cryptonector »

pingyong wrote: post #10

Just looking at the API "sds" seems to just be a typedef for char* - unfortunately, that means that accidentally passing a char* as an sds into any of the functions will be instant UB and not even a compiler warning. Considering this is C, there is no way to prevent this easily since you can't express a type that is one-way convertible (i.e. sds -> char* ok, char* -> sds not ok). I do have to wonder though if avoidin…

The right way to do this would be:

  typedef struct sds {
    char *s;
  } sds;
Now you get type safety: you can't accidentally pass a plain 'ole C string to an sds function. But you do need to access the 's' field to get the C string for passing to non-sds functions.

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#42

Post by chacham15 »

electrograv wrote: post #37
chacham15 wrote: post #25

Earlier quoted context omitted.

What you do is something like the following: typedef struct sds_s { char data[0]; } sds; Which makes them essentially equivalent, but not from a type perspective. Then to handle conversions, you add explicit functions a la char* sds_cstr(sds *str); Then you have full type checking to help you (and an additional advantage if your data format changes).

The downside to using a non-inlined conversion function is that (1) it will be slower than just accessing the member directly, and (2) it’s much more verbose. Why not just use the member explicitly? This way, converting a STS string into a legacy C string could be as simple as writing “.cstr” (or if you just be as terse as possible, it could be defined as “.s” as another poster suggests). In this case, compromise of…

I think you're missing the point here, the conversion function is trivially inlinable:

    char* sds_cstr(sds *str){
        return &str[0]; //you could probably just cast too
    }
The other posters solution is actually not equivalent in this way (his sds is convertable to char* instead of sds*).

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#43

Post by jandrese »

jstimpfle wrote: post #32
Keyframe wrote: post #22

Earlier quoted context omitted.

Unfortunately, yes. Limited usefulness at best without unicode support, at least to a degree. Even UTF-16 or 32 internally would suffice, treating UTF-8 only as ser/de format is good enough these days.

UTF-8 is the preferred internal storage format for most applications. The reason is space efficiency.

That and the perceived simplicity of UTF16/32 is not actually the case so why bother if you have to do it the hard way anyway.

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#45

Post by shawxe »

cryptonector wrote: post #41
pingyong wrote: post #10

Just looking at the API "sds" seems to just be a typedef for char* - unfortunately, that means that accidentally passing a char* as an sds into any of the functions will be instant UB and not even a compiler warning. Considering this is C, there is no way to prevent this easily since you can't express a type that is one-way convertible (i.e. sds -> char* ok, char* -> sds not ok). I do have to wonder though if avoidin…

The right way to do this would be: typedef struct sds { char *s; } sds; Now you get type safety: you can't accidentally pass a plain 'ole C string to an sds function. But you do need to access the 's' field to get the C string for passing to non-sds functions.

This is fine, but it somewhat defeats the purpose of the opaquely prefixed header. I think the idea is just to be able to treat an SDS like a normal string. Unfortunately, there's no way to have that property not be commutative in C.

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#46

Post by liuliu »

saagarjha wrote: post #23
liuliu wrote: post #18

Earlier quoted context omitted.

It is not a real issue in modern days. You just enable address sanitizer in all your debug builds and these issues are obvious at runtime (still not perfect, but an improvement from dark days).

Address sanitizer can only catch errors on branches your debug build exercises.

For redis, I think given the code coverage on test cases, that shouldn't be a concern. TBH, I think any C code that doesn't have a coverage is not a functioning code. Too much bugs can only be caught at runtime (thankful to the sanitizers-family / valgrind).

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#47

Post by cardiffspaceman »

jstimpfle wrote: post #32
Keyframe wrote: post #22

Earlier quoted context omitted.

Unfortunately, yes. Limited usefulness at best without unicode support, at least to a degree. Even UTF-16 or 32 internally would suffice, treating UTF-8 only as ser/de format is good enough these days.

UTF-8 is the preferred internal storage format for most applications. The reason is space efficiency.

I certainly have leaned on UTF-8 but I wonder how efficient it is for the numerically-higher code points if the language is not heavily cp1252, like Korean.

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#48

Post by CJefferson »

antirez wrote: post #24

Hi, author here. May make sense to make SDS in perspective given a few comments I'm reading here. 1. Yep, more than "strings" SDS may be consider a library for dynamic buffers, especially from people coming from C++ or higher level languages. However I think that for C, it makes sense to provide a very low level thing like that. 2. In practice, if you see how SDS is used (extensively) inside Redis, it normally models…

Whenever I've used functionality like this before, the inevitable problem is the string ends up with nulls in it, meaning C and the library disagree on the length of the string, which inevitably causes problems and corruption.

How do you handle this?

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#49

Post by unwind »

rurban wrote: post #20
Keyframe wrote: post #14

UTF support?

Exactly. You should not name a buffer lib "string", when it does not support the basic unicode operations: case fold, normalize => compare, search. In utf-8 of course. I'm also missing stack allocation support, needed for fast short strings. It should be even included in sdsnew, for len < 128.

How do you implement stack allocation support inside a C library function? How is de-allocation expressed?

Re: Simple Dynamic Strings library for C, compatible with null-terminated strings

#50

Post by electrograv »

chacham15 wrote: post #42
electrograv wrote: post #37

Earlier quoted context omitted.

The downside to using a non-inlined conversion function is that (1) it will be slower than just accessing the member directly, and (2) it’s much more verbose. Why not just use the member explicitly? This way, converting a STS string into a legacy C string could be as simple as writing “.cstr” (or if you just be as terse as possible, it could be defined as “.s” as another poster suggests). In this case, compromise of…

I think you're missing the point here, the conversion function is trivially inlinable: char* sds_cstr(sds *str){ return &str[0]; //you could probably just cast too } The other posters solution is actually not equivalent in this way (his sds is convertable to char* instead of sds*).

Even an inlined function only has value if our goal is to open the door to implementation changes in the future. But because the current implementation is zero-cost (an inlined no-op), any behavioral change will necessary compromise performance.

Otherwise (if we want to retain the zero-cost guarantee of converting to a C string), directly accessing the member variable is: functionally equivalent, simpler, more concise, more readable, more explicit.

Post reply on HN