[ccan] [PATCH] stringbuilder: Functions for joining strings.

[Stuart Longland]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 23/08/14 11:40, David Gibson wrote:
>>> Returning an errno code is not a normal error interface.
>>> Usually you'd return -1 (or something negative), and set errno
>>> to EMSGSIZE.
>> 
>> My preference for doing the former is that it's more thread
>> safe, something errno is definitely not.
> 
> So, errno is generally threadsafe these days, albeit via somewhat 
> tortured means.  Generally modern errno will be either a magic
> macro, or (for compilers that support it) a thread local variable,
> which will reference a per-thread errno value.

Ahh okay, you learn something new every day then. :-)

I guess I don't trust errno in a threaded context because of its
history.  Plus, I often code in C on platforms where it probably
*isn't* threadsafe (i.e. bare-iron embedded).

>> I recognise that it's not conventional practice but if I'm going
>> to return one thing for "all okay" and another for "error", then
>> why not return the error code via a route where it's less likely
>> to get splattered by another system call?
> 
> Hrm.  I'm torn.  You're right that returning the error code is a 
> better interface, but I'm not sure that's a strong enough reason
> to break with convention.
> 
> At the very least I'd suggest returning -EWHATEVER, rather than 
> EWHATEVER.  I think when people see a function returning an
> integer status, they're going to expect that (rc < 0) is a suitable
> check for an error condition.

This is doable.  The other thing I can do is do both: set errno *and*
return it (negated if need be).
> [snip]
>>>> + +/* + * Even though stringbuilder is implemented using + *
>>>>  stringbuilder_va, we need to test it explicitly.
>>> 
>>> Really?  Why?
>> 
>> I honestly don't know, but it wasn't happy until I did so. Or
>> maybe I was misinterpreting ccanlint.  It'll give a score, but
>> then leaves me to figure out what I missed.
>> 
>> Perhaps I haven't fully figured out how to drive it yet. :-)
> 
> Hrm, that is very odd.  I wonder if Rusty has any ideas why that
> would be.

I had a closer look, having figured out that I can run ccanlint myself
with -vv as the options, it dumped a lot more helpful information and
I was able to nail that issue.

Turned out it was complaining about the lack of a test case covering
stringbuilder_array with a delimiter.

>>>> +int main(int argc, char *argv[]) +{ +	char string[20]; +
>>>> const char* str_array[] = { +		"xxx", "yyy" +	}; +	int res; +
>>>> +	res = test_stringbuilder_va(NULL, string, sizeof(string), +
>>>> "aaa", "bbb", NULL); +	printf("res: %s, string: %s\n", +
>>>> strerror(res), string); +	ok1(res == 0); +
>>>> ok1(!strcmp(string, "aaabbb"));
>>> 
>>> I suggest using streq() from ccan/str, rather than the
>>> potentially confusing !strcmp construction.
>> 
>> I did try this, but had all kinds of issues with getting the
>> tests to compile.  I don't recall the exact errors I got.
> 
> This is also very strange

If I run ccanlint directly withot ccan/str listed in the dependencies,
I get:
> stringbuilder: Module's CCAN dependencies are the only CCAN files
> #included (depends_accurate): FAIL 
> /home/stuartl/hamradio/hhcp/ccan/ccan/stringbuilder/test/run.c:6:ccan/str
> not listed in _info

But then if I comment out that line and run ccanlint again:
> stringbuilder: Module's CCAN dependencies are the only CCAN files
> #included (depends_accurate): PASS 
> /home/stuartl/hamradio/hhcp/ccan/ccan/stringbuilder/_info:ccan/str
> is an unused dependency

So the advice is contradictory.  What I did in the last patch was to
make my own streq work-alike, ugly sure, but until I get ccanlint
sorted, it works.  I guess a merger with str might solve that problem. :-)

Where ccanlint fails is running `make check-stringbuilder`:
> tools/ccanlint/ccanlint --deps-fail-ignore ccan/build_assert 
> build_assert: Total score: 32/32 make: Circular check-str <-
> check-str dependency dropped. make: *** No rule to make target
> `check-tal/talloc', needed by `check-str'.  Stop.

Regards,
- -- 
Stuart Longland (aka Redhatter, VK4MSL)

I haven't lost my mind...
  ...it's backed up on a tape somewhere.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBCgAGBQJT+BjUAAoJEE36GRQQveO3NhsP/3EXm0YCC3+tkDc1jyCWxc7D
5QKPofETYYELo5TCCBakI9o078uY8MmIodTzoFeQ84CpW87aQSSa3+woyTlNT64f
l9Oxe6rpgfZFy+2OKygV0Pitfj0nAbXOZ4awji+348mWxCmoFVeonWXoqVJ4SbHe
0fFcF8f698s+v08lVcq2b4r9VdO5KwaCzDRIxC5nVMbpv+dhXL3OmCPtV2d9po+R
x1mwHUV72P/sncrA7SkZh5rTcssxZeGzpFJpkyhtsqkudeTXOMRKNVMZXb1WS249
ElpBQOVr96OVW41USZuzK40jAQWcZqBe6Bh/BNIeYy3dRBi6aYj9FFXXDst2ozaS
bVaZQOH8Fb7UFcERxxl2OT0LOaUJbsLUawFY9t19c0mwnvyYlr3WSVjlhiPnmgVe
Kpb0t1iiZfTdS52AJKl7Lzj1zRrFSLCYCT4jhw79RRpcABu14GQunfmzTyOEC0ZY
mpCn+VYUR0aS5fMmnISRYxKR9yXD3eSu2crMeONhXk8yiSYqyKTadwzvl2RnhrVT
0FP7I5nKYu4vMCnPgN5/AxSdoa/6+Bl0wMYP4+SM59A9Mt7Ks5i2wzD+cIoCQc6j
f6/SSkCbccdW3jhs6wk0K0aQ+9/eqjWKvK9wI9Yg83K/3F50Z75PFCiMUF3Fg4KE
lWJCQf9W/X73gcSMpafv
=BEI9
-----END PGP SIGNATURE-----