Skip to content

Commit a4aa777

Browse files
rustyrussellendothermicdev
authored andcommitted
lightningd: create helper routine to make socketpair for a channel.
This is a bit too much boilerplate for these, which mainly do the same thing. We add annotaitons to new_peer_fd so the compiler knows that it cannot return NULL, otherwise with -O3 we get: ``` lightningd/peer_control.c: In function ‘peer_connected_hook_final’: lightningd/peer_control.c:1388:28: error: ‘error’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 1388 | take(towire_connectd_peer_send_msg(NULL, &channel->peer->id, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lightningd/peer_control.c:1313:19: note: ‘error’ was declared here 1313 | const u8 *error; | ^~~~~ lightningd/peer_control.c: In function ‘peer_spoke’: lightningd/peer_control.c:1999:28: error: ‘error’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 1999 | take(towire_connectd_peer_send_msg(NULL, &peer->id, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:311: lightningd/peer_control.o] Error 1 ``` Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 4f7df82 commit a4aa777

File tree

5 files changed

+75
-79
lines changed

5 files changed

+75
-79
lines changed

lightningd/channel_control.c

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,9 @@ static void handle_splice_abort(struct lightningd *ld,
305305
struct bitcoin_outpoint *outpoint;
306306
struct channel_inflight *inflight;
307307
char *reason;
308-
u8 *error;
309-
int fds[2];
308+
const u8 *error;
309+
struct peer_fd *pfd;
310+
int other_fd;
310311

311312
if (!fromwire_channeld_splice_abort(tmpctx, msg, &did_i_abort,
312313
&outpoint, &reason)) {
@@ -346,16 +347,8 @@ static void handle_splice_abort(struct lightningd *ld,
346347
"Restarting channeld after tx_abort on %s channel",
347348
channel_state_name(channel));
348349

349-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
350-
log_broken(channel->log,
351-
"Failed to create socketpair: %s",
352-
strerror(errno));
353-
354-
error = towire_warningfmt(tmpctx, &channel->cid,
355-
"Trouble in paradise?");
356-
log_peer_debug(ld->log, &channel->peer->id,
357-
"Telling connectd to send error %s",
358-
tal_hex(tmpctx, error));
350+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
351+
if (!pfd) {
359352
/* Get connectd to send error and close. */
360353
subd_send_msg(ld->connectd,
361354
take(towire_connectd_peer_send_msg(NULL,
@@ -368,21 +361,17 @@ static void handle_splice_abort(struct lightningd *ld,
368361
peer->connectd_counter)));
369362
return;
370363
}
371-
log_debug(channel->log, "made the socket pair");
372364

373-
if (peer_start_channeld(channel, new_peer_fd(tmpctx, fds[0]), NULL,
374-
false, false)) {
375-
log_info(channel->log, "Sending the peer fd to connectd");
365+
if (peer_start_channeld(channel, pfd, NULL, false, false)) {
376366
subd_send_msg(ld->connectd,
377367
take(towire_connectd_peer_connect_subd(NULL,
378368
&peer->id,
379369
peer->connectd_counter,
380370
&channel->cid)));
381-
subd_send_fd(ld->connectd, fds[1]);
382-
log_info(channel->log, "Sent the peer fd to channeld");
371+
subd_send_fd(ld->connectd, other_fd);
383372
} else {
384373
log_info(channel->log, "peer_start_channeld failed");
385-
close(fds[1]);
374+
close(other_fd);
386375
}
387376
}
388377

lightningd/dual_open_control.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2446,27 +2446,26 @@ json_openchannel_abort(struct command *cmd,
24462446
static char *restart_dualopend(const tal_t *ctx, const struct lightningd *ld,
24472447
struct channel *channel, bool from_abort)
24482448
{
2449-
int fds[2];
2450-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
2451-
log_broken(channel->log,
2452-
"Failed to create socketpair: %s",
2453-
strerror(errno));
2449+
struct peer_fd *pfd;
2450+
int other_fd;
2451+
2452+
pfd = sockpair(tmpctx, channel, &other_fd, NULL);
2453+
if (!pfd)
24542454
return tal_fmt(ctx, "Unable to create socket: %s",
24552455
strerror(errno));
2456-
}
24572456

24582457
if (!peer_restart_dualopend(channel->peer,
2459-
new_peer_fd(tmpctx, fds[0]),
2458+
pfd,
24602459
channel, from_abort)) {
2461-
close(fds[1]);
2460+
close(other_fd);
24622461
return tal_fmt(ctx, "Peer not connected");
24632462
}
24642463
subd_send_msg(ld->connectd,
24652464
take(towire_connectd_peer_connect_subd(NULL,
24662465
&channel->peer->id,
24672466
channel->peer->connectd_counter,
24682467
&channel->cid)));
2469-
subd_send_fd(ld->connectd, fds[1]);
2468+
subd_send_fd(ld->connectd, other_fd);
24702469
return NULL;
24712470
}
24722471

lightningd/peer_control.c

Lines changed: 50 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,26 @@
7373
#include <wire/onion_wire.h>
7474
#include <wire/wire_sync.h>
7575

76+
/* Common pattern: create a sockpair for this channel, return one as a peer_fd */
77+
struct peer_fd *sockpair(const tal_t *ctx, struct channel *channel,
78+
int *otherfd, const u8 **warning)
79+
{
80+
int fds[2];
81+
82+
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
83+
/* Note: preserves errno! */
84+
log_broken(channel->log,
85+
"Failed to create socketpair: %s",
86+
strerror(errno));
87+
if (warning)
88+
*warning = towire_warningfmt(ctx, &channel->cid,
89+
"Trouble in paradise?");
90+
return NULL;
91+
}
92+
*otherfd = fds[1];
93+
return new_peer_fd(ctx, fds[0]);
94+
}
95+
7696
static void destroy_peer(struct peer *peer)
7797
{
7898
peer_node_id_map_del(peer->ld->peers, peer);
@@ -1291,7 +1311,8 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload,
12911311
static void connect_activate_subd(struct lightningd *ld, struct channel *channel)
12921312
{
12931313
const u8 *error;
1294-
int fds[2];
1314+
struct peer_fd *pfd;
1315+
int other_fd;
12951316

12961317
/* If we have a canned error for this channel, send it now */
12971318
if (channel->error) {
@@ -1318,19 +1339,15 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
13181339
case DUALOPEND_OPEN_COMMITTED:
13191340
case DUALOPEND_AWAITING_LOCKIN:
13201341
assert(!channel->owner);
1321-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
1322-
log_broken(channel->log,
1323-
"Failed to create socketpair: %s",
1324-
strerror(errno));
1325-
error = towire_warningfmt(tmpctx, &channel->cid,
1326-
"Trouble in paradise?");
1342+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
1343+
if (!pfd)
13271344
goto send_error;
1328-
}
1345+
13291346
if (peer_restart_dualopend(channel->peer,
1330-
new_peer_fd(tmpctx, fds[0]),
1347+
pfd,
13311348
channel, false))
13321349
goto tell_connectd;
1333-
close(fds[1]);
1350+
close(other_fd);
13341351
return;
13351352

13361353
case CHANNELD_AWAITING_LOCKIN:
@@ -1339,21 +1356,17 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
13391356
case CHANNELD_SHUTTING_DOWN:
13401357
case CLOSINGD_SIGEXCHANGE:
13411358
assert(!channel->owner);
1342-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
1343-
log_broken(channel->log,
1344-
"Failed to create socketpair: %s",
1345-
strerror(errno));
1346-
error = towire_warningfmt(tmpctx, &channel->cid,
1347-
"Trouble in paradise?");
1359+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
1360+
if (!pfd)
13481361
goto send_error;
1349-
}
1362+
13501363
if (peer_start_channeld(channel,
1351-
new_peer_fd(tmpctx, fds[0]),
1364+
pfd,
13521365
NULL, true,
13531366
NULL)) {
13541367
goto tell_connectd;
13551368
}
1356-
close(fds[1]);
1369+
close(other_fd);
13571370
return;
13581371
}
13591372
abort();
@@ -1364,7 +1377,7 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
13641377
&channel->peer->id,
13651378
channel->peer->connectd_counter,
13661379
&channel->cid)));
1367-
subd_send_fd(ld->connectd, fds[1]);
1380+
subd_send_fd(ld->connectd, other_fd);
13681381
return;
13691382

13701383
send_error:
@@ -1838,8 +1851,9 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
18381851
struct channel_id channel_id;
18391852
struct peer *peer;
18401853
bool dual_fund;
1841-
u8 *error;
1842-
int fds[2];
1854+
const u8 *error;
1855+
int other_fd;
1856+
struct peer_fd *pfd;
18431857
char *errmsg;
18441858

18451859
if (!fromwire_connectd_peer_spoke(msg, msg, &id, &connectd_counter, &msgtype, &channel_id, &errmsg))
@@ -1883,18 +1897,13 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
18831897

18841898
log_debug(channel->log, "channel already active");
18851899
if (channel->state == DUALOPEND_AWAITING_LOCKIN) {
1886-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
1887-
log_broken(ld->log,
1888-
"Failed to create socketpair: %s",
1889-
strerror(errno));
1890-
error = towire_warningfmt(tmpctx, &channel_id,
1891-
"Trouble in paradise?");
1900+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
1901+
if (!pfd)
18921902
goto send_error;
1893-
}
1894-
if (peer_restart_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel, false))
1903+
if (peer_restart_dualopend(peer, pfd, channel, false))
18951904
goto tell_connectd;
18961905
/* FIXME: Send informative error? */
1897-
close(fds[1]);
1906+
close(other_fd);
18981907
}
18991908
return;
19001909
}
@@ -1925,19 +1934,13 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
19251934
}
19261935
peer->uncommitted_channel = new_uncommitted_channel(peer);
19271936
peer->uncommitted_channel->cid = channel_id;
1928-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
1929-
log_broken(ld->log,
1930-
"Failed to create socketpair: %s",
1931-
strerror(errno));
1932-
error = towire_warningfmt(tmpctx, &channel_id,
1933-
"Trouble in paradise?");
1937+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
1938+
if (!pfd)
19341939
goto send_error;
1935-
}
1936-
if (peer_start_openingd(peer, new_peer_fd(tmpctx, fds[0]))) {
1940+
if (peer_start_openingd(peer, pfd))
19371941
goto tell_connectd;
1938-
}
19391942
/* FIXME: Send informative error? */
1940-
close(fds[1]);
1943+
close(other_fd);
19411944
return;
19421945

19431946
case WIRE_OPEN_CHANNEL2:
@@ -1950,18 +1953,14 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
19501953
peer->ld->config.fee_base,
19511954
peer->ld->config.fee_per_satoshi);
19521955
channel->cid = channel_id;
1953-
if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
1954-
log_broken(ld->log,
1955-
"Failed to create socketpair: %s",
1956-
strerror(errno));
1957-
error = towire_warningfmt(tmpctx, &channel_id,
1958-
"Trouble in paradise?");
1956+
pfd = sockpair(tmpctx, channel, &other_fd, &error);
1957+
if (!pfd)
19591958
goto send_error;
1960-
}
1961-
if (peer_start_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel))
1959+
1960+
if (peer_start_dualopend(peer, pfd, channel))
19621961
goto tell_connectd;
19631962
/* FIXME: Send informative error? */
1964-
close(fds[1]);
1963+
close(other_fd);
19651964
return;
19661965
}
19671966

@@ -1993,7 +1992,7 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
19931992
take(towire_connectd_peer_connect_subd(NULL, &id,
19941993
peer->connectd_counter,
19951994
&channel_id)));
1996-
subd_send_fd(ld->connectd, fds[1]);
1995+
subd_send_fd(ld->connectd, other_fd);
19971996
}
19981997

19991998
struct disconnect_command {

lightningd/peer_control.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ void channel_errmsg(struct channel *channel,
101101
bool disconnect,
102102
bool warning);
103103

104+
/* Helper to create a peer_fd and an other fd from socketpair.
105+
* Logs error to channel if it fails, and if warning non-NULL, creates
106+
* a warning message */
107+
struct peer_fd *sockpair(const tal_t *ctx, struct channel *channel,
108+
int *otherfd, const u8 **warning);
109+
104110
u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx);
105111
u8 *p2tr_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx);
106112

lightningd/peer_fd.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef LIGHTNING_LIGHTNINGD_PEER_FD_H
22
#define LIGHTNING_LIGHTNINGD_PEER_FD_H
33
#include "config.h"
4+
#include <ccan/compiler/compiler.h>
45
#include <ccan/tal/tal.h>
56

67
/* Tal wrapper for fd connecting subd to connectd */
@@ -10,9 +11,11 @@ struct peer_fd {
1011
};
1112

1213
/* Allocate a new per-peer state and add destructor to close fd if set. */
14+
RETURNS_NONNULL
1315
struct peer_fd *new_peer_fd(const tal_t *ctx, int peer_fd);
1416

1517
/* Array version of above: tal_count(fds) must be 1 */
18+
RETURNS_NONNULL
1619
struct peer_fd *new_peer_fd_arr(const tal_t *ctx, const int *fd);
1720

1821
#endif /* LIGHTNING_LIGHTNINGD_PEER_FD_H */

0 commit comments

Comments
 (0)