Skip to content

Commit

Permalink
refactor: Window ID's and window creation error handling
Browse files Browse the repository at this point in the history
We now use 16-bit ID's for windows and re-use previously discarded
ID's instead of using an incrementer. We also now do proper
error handling on window creation.
  • Loading branch information
JFreegman committed Feb 27, 2024
1 parent 051cdc8 commit 149ef9d
Show file tree
Hide file tree
Showing 16 changed files with 136 additions and 44 deletions.
9 changes: 8 additions & 1 deletion src/audio_call.c
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,14 @@ void callback_recv_invite(Toxic *toxic, uint32_t friend_number)
}

if (Friends.list[friend_number].window_id == -1) {
Friends.list[friend_number].window_id = add_window(toxic, new_chat(toxic->tox, Friends.list[friend_number].num));
const int window_id = add_window(toxic, new_chat(toxic->tox, Friends.list[friend_number].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in callback_recv_invite()\n");
return;
}

Friends.list[friend_number].window_id = window_id;
}

const Call *call = &CallControl.calls[friend_number];
Expand Down
29 changes: 21 additions & 8 deletions src/conference.c
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ static void init_conference_logging(ToxWindow *self, Toxic *toxic, uint32_t conf
}
}

int64_t init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type, const char *title, size_t length)
static void delete_conference(ToxWindow *self, Toxic *toxic, uint32_t conferencenum);

int init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type, const char *title, size_t length)
{
if (toxic == NULL) {
return -1;
Expand All @@ -223,17 +225,32 @@ int64_t init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type,
// probably it so happens that this will (at least typically) be
// the case, because toxic and tox maintain the indices in
// parallel ways. But it isn't guaranteed by the API.
conferences[i].conferencenum = conferencenum;
conferences[i].window_id = add_window(toxic, self);
if (i == max_conference_index) {
++max_conference_index;
}

conferences[i].active = true;
conferences[i].conferencenum = conferencenum;
conferences[i].num_peers = 0;
conferences[i].type = type;
conferences[i].start_time = get_unix_time();
conferences[i].audio_enabled = false;
conferences[i].last_sent_audio = 0;

const int window_id = add_window(toxic, self);

if (window_id < 0) {
fprintf(stderr, "Failed to create new conference window\n");
delete_conference(self, toxic, conferencenum);
return -1;
}

conferences[i].window_id = window_id;

if (!tox_conference_get_id(toxic->tox, conferencenum, (uint8_t *) conferences[i].id)) {
fprintf(stderr, "Failed to fetch conference ID for conferencenum: %u\n", conferencenum);
delete_conference(self, toxic, conferencenum);
return -1;
}

#ifdef AUDIO
Expand All @@ -246,10 +263,6 @@ int64_t init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type,

init_conference_logging(self, toxic, conferencenum);

if (i == max_conference_index) {
++max_conference_index;
}

return conferences[i].window_id;
}
}
Expand Down Expand Up @@ -330,7 +343,7 @@ void conference_rename_log_path(Toxic *toxic, uint32_t conferencenum, const char

if (rename_logfile(toxic->windows, toxic->c_config, chat->title, new_title, myid, conference_id,
chat->window_id) != 0) {
fprintf(stderr, "Failed to rename conference log to `%s`\n", new_title);
fprintf(stderr, "Failed to rename conference log to '%s'\n", new_title);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/conference.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ typedef struct NameListEntry {

typedef struct {
uint32_t conferencenum;
int64_t window_id;
uint16_t window_id;
bool active;
uint8_t type;
int side_pos; /* current position of the sidebar - used for scrolling up and down */
Expand Down Expand Up @@ -90,7 +90,7 @@ typedef struct {
/* Frees all Toxic associated data structures for a conference (does not call tox_conference_delete() ) */
void free_conference(ToxWindow *self, Windows *windows, const Client_Config *c_config, uint32_t conferencenum);

int64_t init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type, const char *title, size_t length);
int init_conference_win(Toxic *toxic, uint32_t conferencenum, uint8_t type, const char *title, size_t length);

/* destroys and re-creates conference window with or without the peerlist */
void redraw_conference_win(ToxWindow *self);
Expand Down
65 changes: 57 additions & 8 deletions src/friendlist.c
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,14 @@ static void friendlist_onMessage(ToxWindow *self, Toxic *toxic, uint32_t num, To
return;
}

Friends.list[num].window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onMessage\n");
return;
}

Friends.list[num].window_id = window_id;
}

static void friendlist_onConnectionChange(ToxWindow *self, Toxic *toxic, uint32_t num, Tox_Connection connection_status)
Expand Down Expand Up @@ -657,7 +664,14 @@ static void friendlist_onGameInvite(ToxWindow *self, Toxic *toxic, uint32_t frie
return;
}

Friends.list[friend_number].window_id = add_window(toxic, new_chat(tox, Friends.list[friend_number].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[friend_number].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onGameInvite\n");
return;
}

Friends.list[friend_number].window_id = window_id;
}

#endif // GAMES
Expand All @@ -680,7 +694,14 @@ static void friendlist_onFileRecv(ToxWindow *self, Toxic *toxic, uint32_t num, u
return;
}

Friends.list[num].window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onFileRecv\n");
return;
}

Friends.list[num].window_id = window_id;
}

static void friendlist_onConferenceInvite(ToxWindow *self, Toxic *toxic, int32_t num, uint8_t type,
Expand All @@ -706,7 +727,14 @@ static void friendlist_onConferenceInvite(ToxWindow *self, Toxic *toxic, int32_t
return;
}

Friends.list[num].window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onConferenceInvite\n");
return;
}

Friends.list[num].window_id = window_id;
}

static void friendlist_onGroupInvite(ToxWindow *self, Toxic *toxic, uint32_t num, const char *data, size_t length,
Expand All @@ -730,7 +758,14 @@ static void friendlist_onGroupInvite(ToxWindow *self, Toxic *toxic, uint32_t num
return;
}

Friends.list[num].window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[num].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onGroupInvite\n");
return;
}

Friends.list[num].window_id = window_id;
}

/* move friendlist/blocklist cursor up and down */
Expand Down Expand Up @@ -1015,7 +1050,14 @@ static bool friendlist_onKey(ToxWindow *self, Toxic *toxic, wint_t key, bool ltr

/* Jump to chat window if already open */
if (Friends.list[f].window_id < 0) {
Friends.list[f].window_id = add_window(toxic, new_chat(tox, Friends.list[f].num));
const int window_id = add_window(toxic, new_chat(tox, Friends.list[f].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onKey\n");
return true;
}

Friends.list[f].window_id = window_id;
}

set_active_window_by_id(toxic->windows, Friends.list[f].window_id);
Expand Down Expand Up @@ -1408,8 +1450,15 @@ static void friendlist_onAV(ToxWindow *self, Toxic *toxic, uint32_t friend_numbe
}

if (state != TOXAV_FRIEND_CALL_STATE_FINISHED) {
Friends.list[friend_number].window_id = add_window(toxic, new_chat(tox, Friends.list[friend_number].num));
set_active_window_by_id(toxic->windows, Friends.list[friend_number].window_id);
const int window_id = add_window(toxic, new_chat(tox, Friends.list[friend_number].num));

if (window_id < 0) {
fprintf(stderr, "Failed to create new chat window in friendlist_onAV");
return;
}

Friends.list[friend_number].window_id = window_id;
set_active_window_by_id(toxic->windows, window_id);
}
}
#endif /* AUDIO */
Expand Down
2 changes: 1 addition & 1 deletion src/friendlist.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ typedef struct {
size_t statusmsg_len;
char pub_key[TOX_PUBLIC_KEY_SIZE];
uint32_t num;
int64_t window_id;
int window_id;
bool active;
Tox_Connection connection_status;
bool is_typing;
Expand Down
2 changes: 1 addition & 1 deletion src/game_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ int game_initialize(const ToxWindow *parent, Toxic *toxic, GameType type, uint32

GameData *game = self->game;

const int64_t window_id = add_window(toxic, self);
const int window_id = add_window(toxic, self);

if (window_id < 0) {
free(game);
Expand Down
2 changes: 1 addition & 1 deletion src/game_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ struct GameData {
int parent_max_x; // max dimensions of parent window
int parent_max_y;

int64_t window_id;
uint16_t window_id;
WINDOW *window;

Toxic *toxic; // must be locked with Winthread mutex
Expand Down
2 changes: 1 addition & 1 deletion src/global_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ void cmd_conference(WINDOW *window, ToxWindow *self, Toxic *toxic, int argc, cha
#endif
}

if (init_conference_win(toxic, conferencenum, type, NULL, 0) == -1) {
if (init_conference_win(toxic, conferencenum, type, NULL, 0) < 0) {
line_info_add(self, c_config, false, NULL, NULL, SYS_MSG, 0, 0, "Conference window failed to initialize.");
tox_conference_delete(tox, conferencenum, NULL);
return;
Expand Down
18 changes: 14 additions & 4 deletions src/groupchats.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,19 +387,29 @@ int init_groupchat_win(Toxic *toxic, uint32_t groupnumber, const char *groupname

for (int i = 0; i <= max_groupchat_index; ++i) {
if (!groupchats[i].active) {
groupchats[i].window_id = add_window(toxic, self);
if (i == max_groupchat_index) {
++max_groupchat_index;
}

groupchats[i].active = true;
groupchats[i].groupnumber = groupnumber;
groupchats[i].num_peers = 0;
groupchats[i].time_connected = get_unix_time();

if (!tox_group_get_chat_id(tox, groupnumber, (uint8_t *) groupchats[i].chat_id, NULL)) {
const int window_id = add_window(toxic, self);

if (window_id < 0) {
fprintf(stderr, "Failed to create new groupchat window\n");
close_groupchat(self, toxic, groupnumber);
return -1;
}

if (i == max_groupchat_index) {
++max_groupchat_index;
groupchats[i].window_id = window_id;

if (!tox_group_get_chat_id(tox, groupnumber, (uint8_t *) groupchats[i].chat_id, NULL)) {
fprintf(stderr, "Failed to fetch new groupchat ID\n");
close_groupchat(self, toxic, groupnumber);
return -1;
}

set_active_window_by_id(toxic->windows, groupchats[i].window_id);
Expand Down
2 changes: 1 addition & 1 deletion src/groupchats.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ typedef struct {
bool active;
uint64_t time_connected; /* The time we successfully connected to the group */

int64_t window_id;
uint16_t window_id;
int side_pos; /* current position of the sidebar - used for scrolling up and down */
} GroupChat;

Expand Down
2 changes: 1 addition & 1 deletion src/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ int load_chat_history(struct chatlog *log, ToxWindow *self, const Client_Config
* Return -1 on failure.
*/
int rename_logfile(Windows *windows, const Client_Config *c_config, const char *src, const char *dest,
const char *selfkey, const char *otherkey, uint32_t window_id)
const char *selfkey, const char *otherkey, uint16_t window_id)
{
ToxWindow *toxwin = get_window_pointer_by_id(windows, window_id);
struct chatlog *log = NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/log.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,6 @@ int load_chat_history(struct chatlog *log, ToxWindow *self, const Client_Config
* Return -1 on failure.
*/
int rename_logfile(Windows *windows, const Client_Config *c_config, const char *src, const char *dest,
const char *selfkey, const char *otherkey, uint32_t window_id);
const char *selfkey, const char *otherkey, uint16_t window_id);

#endif /* LOG_H */
4 changes: 2 additions & 2 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,9 @@ static void load_conferences(Toxic *toxic)

title[length] = 0;

const int64_t win_id = init_conference_win(toxic, conferencenum, type, (const char *) title, length);
const int win_id = init_conference_win(toxic, conferencenum, type, (const char *) title, length);

if (win_id == -1) {
if (win_id < 0) {
tox_conference_delete(tox, conferencenum, NULL);
continue;
}
Expand Down
1 change: 0 additions & 1 deletion src/toxic.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ typedef struct Windows {
ToxWindow **list;
uint16_t count;
uint16_t active_index;
uint32_t next_id;
} Windows;

typedef struct Toxic {
Expand Down
21 changes: 14 additions & 7 deletions src/windows.c
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ void on_group_voice_state(Tox *tox, uint32_t groupnumber, Tox_Group_Voice_State
* Returns the windows list index of the window associated with `id`.
* Returns -1 if `id` is not found.
*/
static int get_window_index(Windows *windows, uint32_t id)
static int get_window_index(Windows *windows, uint16_t id)
{
for (uint16_t i = 0; i < windows->count; ++i) {
if (windows->list[i]->id == id) {
Expand All @@ -772,16 +772,23 @@ static int get_window_index(Windows *windows, uint32_t id)
return -1;
}

static uint32_t get_new_window_id(Windows *windows)
static uint16_t get_new_window_id(Windows *windows)
{
const uint32_t new_id = windows->next_id;
++windows->next_id;
uint16_t new_id = 0;

for (uint16_t i = 0; i < UINT16_MAX; ++i) {
if (get_window_pointer_by_id(windows, i) == NULL) {
new_id = i;
break;
}
}

return new_id;
}

/* CALLBACKS END */

int64_t add_window(Toxic *toxic, ToxWindow *w)
int add_window(Toxic *toxic, ToxWindow *w)
{
if (w == NULL || LINES < 2) {
fprintf(stderr, "Failed to add window.\n");
Expand Down Expand Up @@ -835,7 +842,7 @@ void set_active_window_by_type(Windows *windows, Window_Type type)
fprintf(stderr, "Warning: attemping to set active window with no active type: %d\n", type);
}

void set_active_window_by_id(Windows *windows, uint32_t id)
void set_active_window_by_id(Windows *windows, uint16_t id)
{
const int idx = get_window_index(windows, id);

Expand Down Expand Up @@ -1357,7 +1364,7 @@ void refresh_inactive_windows(Windows *windows, const Client_Config *c_config)
/* Returns a pointer to the ToxWindow associated with `id`.
* Returns NULL if no ToxWindow exists.
*/
ToxWindow *get_window_pointer_by_id(Windows *windows, uint32_t id)
ToxWindow *get_window_pointer_by_id(Windows *windows, uint16_t id)
{
for (uint16_t i = 0; i < windows->count; ++i) {
ToxWindow *w = windows->list[i];
Expand Down
Loading

0 comments on commit 149ef9d

Please sign in to comment.