Clever or tidy code snippets

[Frederick Gotham]

I came across this piece of C++ code yesterday for running a separate program on Linux and getting its stdout. It uses "popen" to start the second program, and later uses "pclose" to close the stream.

unique_ptr<FILE, decltype(&pclose)> stdout_stream(popen("echo Hello World"),"r"), pclose);

I thought it was pretty neat.

[Jorgen Grahn]

It shows what you can do with std::unique_ptr so yes it's kind of
neat. But you lose the ability to check the exit status of the other
process, and that's often not acceptable.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Frederick Gotham]

On Tuesday, March 3, 2020 at 3:21:06 PM UTC, Jorgen Grahn wrote:
> On Tue, 2020-03-03, Frederick Gotham wrote:
> >
> > I came across this piece of C++ code yesterday for running a
> > separate program on Linux and getting its stdout. It uses "popen" to
> > start the second program, and later uses "pclose" to close the
> > stream.
> >
> > unique_ptr<FILE, decltype(&pclose)> stdout_stream(popen("echo Hello World"),"r"), pclose);
> >
> > I thought it was pretty neat.
>
> It shows what you can do with std::unique_ptr so yes it's kind of
> neat. But you lose the ability to check the exit status of the other
> process, and that's often not acceptable.

It's fine in my case as I only need the string (which can be empty if the second program crashes).

Code taken from stackoverflow:

#include <cstdio> /* In Linux this contains popen, pclose */
#include <iostream>
#include <memory>
#include <stdexcept>
#include <string>
#include <array>

std::string exec(char const *const cmd)
{
std::array<char, 128> buffer;

std::string result;

std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd, "r"), pclose);

if ( !pipe )
{
throw std::runtime_error("popen() failed!");
}

while ( nullptr != fgets(buffer.data(), buffer.size(), pipe.get()) )
{
result += buffer.data();
}

return result;
}

[Maciej Sobczak]

> It's fine in my case as I only need the string (which can be empty if the second program crashes).

> while ( nullptr != fgets(buffer.data(), buffer.size(), pipe.get()) )

And what if the second program crashes in the middle of producing the output? Is the possibility to read partial (corrupted?) output acceptable for you?

This code example is indeed neat, but also a bit deceptive. It makes provisions for the popen failing (and translates it into exception), but ignores the possibility of fgets failing sometime later (also by means other than EOF).
It might be negligible in your particular case (the result might have some integrity features built in its syntax), but is worth being aware of before copy-pasting this code to some other arbitrary system.

But it's true, custom deleters in smart pointers are indeed powerful. :-)

--
Maciej Sobczak * http://www.inspirel.com

[cda...@gmail.com]

Ugh...does every other person feel the need to use this idiotic construct? By that way, this can cause undefined behavior.

And with, I'm adding the next person to my killfile who thinks this construct is okay and then tries to justify it with some lame, divorced from reality excuse.

[cda...@gmail.com]

And the code can also produce undefined behavior. More to the point, I can think of a case where this idiot construct would evaluate to true when the expression is actually false.

[cda...@gmail.com]

The ultimate moral is, don't copy and paste things from Stackoverflow unless you know the person's credentials. Because in this case, the person on Stackoverflow who posted this code needs to stay as a Junior Software Developer or the tech support person.

[Öö Tiib]

What kind of undefined behavior? What construct?
Usage of unique_ptr for simple RAII cleanup is well-known idiom.
No point to make full-blown wrapper on every case.
Launcing sub-processes or opening external communication
channels should not be taken that lightly however. Such
std::unique_ptr<FILE, std::function<void(FILE*)>> feels too
naive there.

[cda...@gmail.com]

This line...

while ( nullptr != fgets(buffer.data(), buffer.size(), pipe.get()) )

If nullptr returns a value, the entire expression

nullptr != fgets(buffer.data(), buffer.size(), pipe.get())

could get evaluated. Whereas if nullptr was was on the other side, it will throw a runtime error(?). I think that's how it's worded. The only reason I know this is because I've dealt with a class of bugs similar to this. And it *always* stems from "inverting" the expression.

[Öö Tiib]

Uh, I don't follow. The nullptr is a keyword of C++ that can be used
as null pointer constant expression. It was added to make sure that
it can't be implicitly converted to or compared with integer constant
(the issue that NULL had).

The fgets is documented to return null pointer when there was end of
file before reading any characters or error occurred during reading.
So the line is technically just more explicit way to write:

while ( fgets(buffer.data(), buffer.size(), pipe.get()) )

> Whereas if nullptr was was on the other side, it will throw a runtime error(?). I think that's how it's worded. The only reason I know this is because I've dealt with a class of bugs similar to this. And it *always* stems from "inverting" the expression.

How it can throw? The fgets is C library function so noexcept and
inbuilt operator != between two pointers is also noexcept. User
has to call ferror(pipe.get()) to find out if there was error or
not, nothing should throw there.

[Frederick Gotham]

On Wednesday, March 4, 2020 at 6:00:50 PM UTC, Öö Tiib wrote:

> > If nullptr returns a value, the entire expression
> >
> > nullptr != fgets(buffer.data(), buffer.size(), pipe.get())
> >
> > could get evaluated.
>
> Uh, I don't follow.

Sometimes misinformation is a little too perfect to be accidental.

[cda...@gmail.com]

If nullptr is on the left hand side, it evaluates to a value, and thus, the entire expression will evaluate right away. However, if nullptr is on the right hand side, it might not evaluate right away. Thus the former can lead to a subtle bug.

[James Kuyper]

You might be right. But that message was posted using Chad's e-mail
address, so I think that amazing amounts of confusion, possibly fueled
by substance abuse, is also a plausible explanation.

[James Kuyper]

And what consequence do you think evaluation of nullptr will have that
could cause this bug? Keep in mind that evaluation of nullptr does not
(in this context) mean anything different from evaluation of NULL or 0
or (char*)0. That evaluation doesn't actually do anything that could
interfere with anything else.

The evaluation of the left operand of a comparison operator is
indeterminately sequenced with respect to evaluation of the right
operand, and there's no sequence point separating them. Therefore, if
expression1 != expression2 is a problem because it allows the two
expressions to be evaluated in a problematic order, then expression2 !=
expression1 is just as much as problem, because an implementation is
allowed to evaluate them in the same problematic order with either
version of that expression.

But I still have no idea why you think it's a problem.

[David Brown]

In order to clear up your confusion, please tell us what it is you don't
understand about C++ here:

1. The != operator, like every binary operator other than the comma
operator, && and ||, requires that both sides are always evaluated
before the operator is applied.

2. For the != operator, like all binary operators except the three
mentioned above, the order of evaluation is unspecified.

Given "A != B", the compiler can choose to evaluate A first, then B, or
B first then A, or to intermingle the evaluation of A and B. But both A
and B must be fully evaluated before comparing their values.

3. "nullptr" is a keyword in C++. It does not need evaluating as such,
any more than the literal 0 needs evaluating - it certainly has no
side-effects.

[James Kuyper]

Let me expand on what I said earlier. The line of code in question is

while ( nullptr != fgets(buffer.data(), buffer.size(), pipe.get()) )

The != operator is symmetric, and evaluations of the the operands of !=
are unsequenced (4.6p17) (I earlier said they were indeterminately
sequenced, but that was a mistake), so the set of permitted behaviors is
the same whether you write it that way or

fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr.

Given that it's being compared with a pointer, nullptr plays no role in
this expression that is any different from any other null pointer
constant (7.11). Therefore, any problems that might exist in this
expression would also be present if nullptr were replaced with NULL or 0.
In a context such as this, any null pointer constant gets implicitly
converted to a null pointer of the same pointer type as the other
operand, in this case char*. Therefore, whatever problems this
expression might have would be unchanged if you replace nullptr with
(char*)0.
A null pointer compares equal to all other null pointers, and unequal to
any other pointers (8.10p2). An expression which appears as a condition
in a while statement gets contextually converted to bool (9p4). The
result of converting a null pointer to bool is false, and any other
pointer value converts to true (7.14p1), so

while(nullptr != fgets(buffer.data(), buffer.size(), pipe.get()))

has the same exact behavior as

while( fgets(buffer.data(), buffer.size(), pipe.get()).

Therefore, if there's any problem with the original form of this code,
there should still be a problem that form.

Could you please identify what you think that problem might be?

[Chris M. Thomasson]

On 3/6/2020 6:28 AM, James Kuyper wrote:
> On Friday, March 6, 2020 at 8:55:41 AM UTC-5, cda...@gmail.com wrote:

> And what consequence do you think evaluation of nullptr will have that
> could cause this bug? Keep in mind that evaluation of nullptr does not
> (in this context) mean anything different from evaluation of NULL or 0
> or (char*)0. That evaluation doesn't actually do anything that could
> interfere with anything else.

> 123456789012345678901234567890123456789012345678901234567890123456789012

> The evaluation of the left operand of a comparison operator is
> indeterminately sequenced with respect to evaluation of the right
> operand, and there's no sequence point separating them. Therefore, if
> expression1 != expression2 is a problem because it allows the two
> expressions to be evaluated in a problematic order, then expression2 !=
> expression1 is just as much as problem, because an implementation is
> allowed to evaluate them in the same problematic order with either
> version of that expression.
>
> But I still have no idea why you think it's a problem.
>

Neither do I.

[Chris M. Thomasson]

Why?

[cda...@gmail.com]

Here is something from a beta Java project that I'm working on..

https://github.com/cdalten/WorkReminder/blob/master/app/src/main/java/com/example/cd/workreminder/BootReceiver.java

If I would invert the expression, I wouldn't get a possible error when the device boots up.

[cda...@gmail.com]

This is in reference to line 39.

[James Kuyper]

On Friday, March 6, 2020 at 7:00:51 PM UTC-5, cda...@gmail.com wrote:
> This is in reference to line 39.

Line 39 is:

if (intent.getAction().equals("android.intent.action.BOOT_COMPLETED"))


That code is java, not C++ - just because something would be a problem
in Java doesn't tell you whether or not it would be a problem with C++.
I'm not particularly familiar with Java, and therefore anything I say
about that code below might be irrelevant - to Java. But this is a C++
newsgroup, and in C++ a member function like equals() doesn't have the
same kind of symmetry that the != operator has. As a result, it's not at
all clear what change to the code you have in mind when you talk about
inverting the expression. Depending upon the actual semantics of those
member functions, it's entirely plausible that there might be some good
reason why you would need to invert the expression to get good results.
But whatever that reason might be, it doesn't apply to the C++ code that
we've been talking about.

[cda...@gmail.com]

Inverting the expression I mean having

if (intent.getAction().equals("android.intent.action.BOOT_COMPLETED"))

as

if ("android.intent.action.BOOT_COMPLETED").equals(intent.getAction())

Is a bad practice. Because in the above case, the first catches the error at runtime while the other doesn't. While this might be okay in C++, carry this kind of practice over to other programming languages has some really really bad consequences.

[Chris M. Thomasson]

Fwiw, in C++:

(nullptr != function())

and:

(function() != nullptr)

will give the same results.

[Chris M. Thomasson]

On 3/7/2020 4:05 PM, Chris M. Thomasson wrote:
> On 3/7/2020 5:47 AM, cda...@gmail.com wrote:
>> On Friday, March 6, 2020 at 8:34:18 PM UTC-8, James Kuyper wrote:
>>> On Friday, March 6, 2020 at 7:00:51 PM UTC-5, cda...@gmail.com wrote:

[...]

> Fwiw, in C++:
>
> (nullptr != function())
>
> and:
>
> (function() != nullptr)
>
> will give the same results.

#include <iostream>

void* function()
{
return nullptr;
}

int main()
{
bool a = (function() != nullptr);
bool b = (nullptr != function());

std::cout << "a = " << a << ", b = " << b << "\n";

return 0;
}

[James Kuyper]

I know very little about java, other than that, in some ways, it looks a
lot like C++. Would you care to explain, in terms suitable to my level
of java expertise, precisely what the two forms do, and why it is that
only one of the two forms catches the error?

My naive expectation would be that both forms retrieve from "intent"
something that is called an Action, and that an Action either is, or at
least, can be compared with, a character string. Then both forms carry
out precisely such a comparison, returning some kind of boolean result.
It's not clear to me how this code has anything to do with "catching an
error", nor why the two forms might differ in their ability to do so.

[Pavel]

IIRC if getAction returns null,

"androind.intent.action.BOOT_COMPLETED".equals(intent.getAction())

won't throw, just returns false; but

intent.getAction().equals("android.intent.action.BOOT_COMPLETED")

will throw.

Usually, you would do something like class Util { public static AreEqual(Object
o1, Object o2) {/*obvious 9 yards*/} } and then use Util.AreEqual instead of
equals if you want null and non-null references to be comparable.

-Pavel

[Jorgen Grahn]

Ah, cdalten's argument finally makes sense. IMO, that is caused by
things in Java which look like objects really being more like
pointers, which may be null. And apparently support for that in
library functions, so that String.equals(null) == false.

That's IMO a weirdness in Java which shouldn't affect how we write
C++ code. We're responsible for not creating null references and
for not dereferencing null pointers.

(I dislike 'if (nullptr == expr)' anyway, but for different reasons.)

> Usually, you would do something like class Util { public static AreEqual(Object
> o1, Object o2) {/*obvious 9 yards*/} } and then use Util.AreEqual instead of
> equals if you want null and non-null references to be comparable.

But it seems here the String class tries to be helpful; no doubt most
people will use that rather than write wrappers.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[James Kuyper]

Ah, that makes sense now. In C++, you could get the same problem by
providing a function with two (or more) overloads:

bool operator==(const A*left, const B*right) {
if(!left) throw();
if(!right) return false;
// compare *left with *right
}

and

bool operator==(const B*left, const A*right) {
if(!left) throw();
if(!right) return false;
// compare *left with *right
}

The syntax for using such overloads would be quite different from the
syntax of his Java example. However, to be fair, the syntax of his Java
example was quite different from the syntax of the C++ code he
originally objected to as

However, while C++ is flexible enough to reproduce this problem, writing
C++ code that way wouldn't be at all reasonable. First of all, one would
ordinarily use references, not pointers, which wouldn't allow you to do
anything of the kind. Secondly, written this way, the asymmetry of the
two overloads sticks out like a sore thumb - it's implausible that it
would never get

> That's IMO a weirdness in Java which shouldn't affect how we write
> C++ code. We're responsible for not creating null references and
> for not dereferencing null pointers.

Agreed.

[Chris M. Thomasson]

Perfect.

>
> That's IMO a weirdness

Agreed.

[Chris M. Thomasson]

Okay.

but
>
> intent.getAction().equals("android.intent.action.BOOT_COMPLETED")
>
> will throw.

A little odd to me. intent.getAction().equals will throw on null, yet
the equals function for the string literal will not because its just
using intent.GetAction(), nothing more. I see it, but it seems odd. the
equals function behaves differently. Thanks.

[red floyd]

On 3/8/20 1:01 AM, Jorgen Grahn wrote:

>
> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
>

I agree, it doesn't "read" properly. I understand what's going on,
though. I think it's a protection against typo for a single = sign,
if expr happens to evaluate to an lvalue of some kind.

e.g.:

T*& f();

if (f() = nullptr) // oops wrong operator! No diagnostic required.

if (nullptr = T()) // oops wrong operator! ERROR

[David Brown]

That is sometimes known as a "Yoda conditional", precisely because a lot
of people feel it reads backwards. Typos here are easily found by many
tools (IDE's, linters, and compilers) - but if you enable warnings on
"if (x = 0)" then you need extra parenthesis for when you really want to
check the result of an assignment, as "if (x = next_item())" will give
the same warning. But it is very common to have such warnings enabled.

However, Jorgen may also have been referring to the superfluous
comparison to nullptr. If "expr" has a pointer type, then "if (expr)"
has exactly the same effect as comparing it to "nullptr".

[Jorgen Grahn]

On Mon, 2020-03-09, red floyd wrote:
> On 3/8/20 1:01 AM, Jorgen Grahn wrote:
>
>> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
>>
> I agree, it doesn't "read" properly. I understand what's going on,
> though. I think it's a protection against typo for a single = sign,
> if expr happens to evaluate to an lvalue of some kind.

That whole thing (that argument, the counter-argument, the counter-
counter-arguments) has been beaten to death, though. I hope I didn't
reanimate it.

[Öö Tiib]

On Monday, 9 March 2020 09:45:35 UTC+2, Jorgen Grahn wrote:
> On Mon, 2020-03-09, red floyd wrote:
> > On 3/8/20 1:01 AM, Jorgen Grahn wrote:
> >
> >> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
> >>
> > I agree, it doesn't "read" properly. I understand what's going on,
> > though. I think it's a protection against typo for a single = sign,
> > if expr happens to evaluate to an lvalue of some kind.
>
> That whole thing (that argument, the counter-argument, the counter-
> counter-arguments) has been beaten to death, though. I hope I didn't
> reanimate it.

I would not participate in project that regulates such idiocy one way or
other. Also I would not let operator== or operator!= overloads that do
not work symmetrically and commutatively to pass review.

[Jorgen Grahn]

On Mon, 2020-03-09, David Brown wrote:
...

> However, Jorgen may also have been referring to the superfluous
> comparison to nullptr.

I had forgotten when I responded earlier but yes, I did. I happily
use pointers and smart pointers in boolean context.

[David Brown]

On 09/03/2020 09:04, Öö Tiib wrote:
> On Monday, 9 March 2020 09:45:35 UTC+2, Jorgen Grahn wrote:
>> On Mon, 2020-03-09, red floyd wrote:
>>> On 3/8/20 1:01 AM, Jorgen Grahn wrote:
>>>
>>>> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
>>>>
>>> I agree, it doesn't "read" properly. I understand what's going on,
>>> though. I think it's a protection against typo for a single = sign,
>>> if expr happens to evaluate to an lvalue of some kind.
>>
>> That whole thing (that argument, the counter-argument, the counter-
>> counter-arguments) has been beaten to death, though. I hope I didn't
>> reanimate it.
>
> I would not participate in project that regulates such idiocy one way or
> other.

Not everyone has that choice. There are some coding standards used that
dictate (or at least strongly encourage) "Yoda conditionals". (I'm sure
you'd dislike other rules too, and for equally good reasons.) Most
programmers work on the projects they are paid to work on and told to
work on, rather than projects they want to work on.

> Also I would not let operator== or operator!= overloads that do
> not work symmetrically and commutatively to pass review.
>

I can't imagine any coding standard that would not agree with that one!

[Bo Persson]

On 2020-03-09 at 10:54, David Brown wrote:
> On 09/03/2020 09:04, Öö Tiib wrote:
>> On Monday, 9 March 2020 09:45:35 UTC+2, Jorgen Grahn wrote:
>>> On Mon, 2020-03-09, red floyd wrote:
>>>> On 3/8/20 1:01 AM, Jorgen Grahn wrote:
>>>>
>>>>> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
>>>>>
>>>> I agree, it doesn't "read" properly. I understand what's going on,
>>>> though. I think it's a protection against typo for a single = sign,
>>>> if expr happens to evaluate to an lvalue of some kind.
>>>
>>> That whole thing (that argument, the counter-argument, the counter-
>>> counter-arguments) has been beaten to death, though. I hope I didn't
>>> reanimate it.
>>
>> I would not participate in project that regulates such idiocy one way or
>> other.
>
> Not everyone has that choice. There are some coding standards used that
> dictate (or at least strongly encourage) "Yoda conditionals". (I'm sure
> you'd dislike other rules too, and for equally good reasons.) Most
> programmers work on the projects they are paid to work on and told to
> work on, rather than projects they want to work on.
>

And some programmers go looking for another employer if the work gets
too silly.

[David Brown]

And some programmers can't conveniently do so. We could go round in
circles all day. But I think it is undeniable that any professional is
going to have to spend some time doing work he/she doesn't like,
following rules that they think are wrong, spending effort that they
think is counter-productive. Those that are lucky and/or higher up in
the ranks do this less, those that are unlucky do it more. There are
certainly very few who would be ready to walk out of a job because they
are asked to write "if (1 == x)" instead of "if (x == 1)".

[James Kuyper]

On Friday, March 7, 2020 at 05:47:54 UTC-8, Chad wrote:
...

> Inverting the expression I mean having
>
> if (intent.getAction().equals("android.intent.action.BOOT_COMPLETED"))
>
> as
>
> if ("android.intent.action.BOOT_COMPLETED").equals(intent.getAction())
>
> Is a bad practice. Because in the above case, the first catches the
> error at runtime while the other doesn't. While this might be okay in
> C++, carry this kind of practice over to other programming languages
> has some really really bad consequences.

No, this kind of practice would not be OK in C++. But when I say "this
kind of practice", I'm not referring to performing the comparison in the
"wrong" order. There shouldn't be a "wrong" order for an inherently
symmetric operator like equals() (or != in the C++ code you were
originally complaining about). The thing that is not OK about this code
is defining equals() in such a way that the order makes a difference.

Thanks to Jorgen, I now understand that this behavior depends upon the
fact that, unlike C++, Java supports null references. In C++, in order
for getAction() to indicate that there was no action to be gotten, it
would have to return a pointer, not a reference (or possibly
std::optional<> - but I'm not sufficiently familiar with std::optional<>
to be sure of that). Null pointers ARE supported in C++. You certainly
could define a comparison that takes pointer arguments, and compares the
things pointed at, rather than the pointers themselves. You could define
the function to either throw or return false when one of the pointers is
null - either behavior might make sense, in different contexts.

However, it is bad design to define equals() in such a way that, when pB
is null, equals(pA, pB) throws while equals(pB, pA) returns false. And
that's what's not OK about the java code you've shown. The C++ code you
claimed had undefined behavior displays no such problem.

[Öö Tiib]

On Monday, 9 March 2020 11:54:32 UTC+2, David Brown wrote:
> On 09/03/2020 09:04, Öö Tiib wrote:
> > On Monday, 9 March 2020 09:45:35 UTC+2, Jorgen Grahn wrote:
> >> On Mon, 2020-03-09, red floyd wrote:
> >>> On 3/8/20 1:01 AM, Jorgen Grahn wrote:
> >>>
> >>>> (I dislike 'if (nullptr == expr)' anyway, but for different reasons.)
> >>>>
> >>> I agree, it doesn't "read" properly. I understand what's going on,
> >>> though. I think it's a protection against typo for a single = sign,
> >>> if expr happens to evaluate to an lvalue of some kind.
> >>
> >> That whole thing (that argument, the counter-argument, the counter-
> >> counter-arguments) has been beaten to death, though. I hope I didn't
> >> reanimate it.
> >
> > I would not participate in project that regulates such idiocy one way or
> > other.
>
> Not everyone has that choice. There are some coding standards used that
> dictate (or at least strongly encourage) "Yoda conditionals". (I'm sure
> you'd dislike other rules too, and for equally good reasons.) Most
> programmers work on the projects they are paid to work on and told to
> work on, rather than projects they want to work on.

I dislike deliberate usage of assignments in conditionals that are
actually confusing ... like if(p=getP()). The if(getP()==p) or
if(p==getP()) are both fine C++ for me.

Probable typos in usage of operators = and == are diagnosed by
majority of compilers and by lot of other tools. When reasonable
proposals of discarding pointless rule and to set up tool-chain
properly instead are rejected then it is not project for me. I am
more profitable participant in other projects. Other people may be are
different and that is normal.