Skip to content

Commit

Permalink
Added assert()s on a lot of NULL parameters. In theory, you should be…
Browse files Browse the repository at this point in the history 
… able to call most functions in Curses with NULLs and just get an error code back. But if you do so, it will almost invariably indicate a bug that ought to result in bombing out so you'll know about it and fix it. Such cases should not be harmlessly elided, resulting in crashes later on that leave you scratching your head.
  • Loading branch information
@Bill-Gray
Bill-Gray committed Oct 4, 2020
1 parent 0623c44 commit 96f4984
Show file tree
Hide file tree
Showing 25 changed files with 138 additions and 0 deletions.
2 changes: 2 additions & 0 deletions pdcurses/addch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ int wadd_wch(WINDOW *win, const cchar_t *wch)
{
PDC_LOG(("wadd_wch() - called: win=%p wch=%x\n", win, *wch));

assert( wch);
return wch ? waddch(win, *wch) : ERR;
}

Expand Down Expand Up @@ -687,6 +688,7 @@ int wecho_wchar(WINDOW *win, const cchar_t *wch)
{
PDC_LOG(("wecho_wchar() - called: win=%p wch=%x\n", win, *wch));

assert( wch);
if (!wch || (wadd_wch(win, wch) == ERR))
return ERR;

Expand Down
3 changes: 3 additions & 0 deletions pdcurses/addchstr.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -74,6 +75,8 @@ int waddchnstr(WINDOW *win, const chtype *ch, int n)

PDC_LOG(("waddchnstr() - called: win=%p n=%d\n", win, n));

assert( win);
assert( ch);
if (!win || !ch || !n || n < -1)
return ERR;

Expand Down
5 changes: 5 additions & 0 deletions pdcurses/addstr.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -69,6 +70,8 @@ int waddnstr(WINDOW *win, const char *str, int n)

PDC_LOG(("waddnstr() - called: string=\"%s\" n %d \n", str, n));

assert( win);

This comment has been minimized.

Sign in to view

Copy link
@GitMensch

GitMensch Dec 15, 2021

Collaborator

@Bill-Gray Isn't that duplicate now (here and in the change below) - first the assert, then the check -> ERR?

This comment has been minimized.

Sign in to view

Copy link
@Bill-Gray

Bill-Gray Dec 15, 2021

Author Owner

It's redundant in debug builds. In release builds, assert doesn't do anything.

According to specification, you're supposed to be able to pass NULL to these functions. Seems bizarre to me, but I'm a couple of decades late in arguing the point. It did (and does) seem reasonable to me to bomb out in debug builds; I still don't see a case where you'd pass NULL without it indicating a bug. (Counterexamples welcomed.)

This comment has been minimized.

Sign in to view

Copy link
@GitMensch

GitMensch Dec 15, 2021

Collaborator

How do you specify a release build with GCC? For MSVC "the internet" says -DNDEBUG, but for others - no idea.
If even the specs say you are allowed to pass a NULL then people will likely find this a "breaking stable API change" - the manpages I've looked at did not explicit say it is allowed (they just didn't said anything other then "on error ERR is returned", which could be read as "no assert should ever happen" [but passing NUL will commonly break things, so...]).

We actually had someone that reported "some strange assert happens in my GnuCOBOL when using newer PDCursesMod versions" (but when the COBOL parts were compiled with runtime checks enabled [per COBOL standard that is off by default] then the program gets a "clean" COBOL abort before addnstr is called, so I don't consider it a "bug" in PDCurses but the program which uses it.

This comment has been minimized.

Sign in to view

Copy link
@Bill-Gray

Bill-Gray Dec 15, 2021

Author Owner

It's -DNDEBUG for gcc, too.

You're definitely allowed to send NULL to (at least most) of these functions; their specifications state explicitly that you'll just get an error code returned. I suppose this does mean that debug builds now break the API... but I'd still maintain that getting warned you have a serious bug is far better than simply sweeping that bug under the rug.

I could imagine having a -DPDC_ALLOW_NULL_POINTERS switch. But I would first want to see even one instance where a NULL pointer being passed to these functions didn't indicate a bug that ought to be fixed. Even then, it should ideally lead to the question "this is a truly horrible idea; are you sure you want to do this? [y/N]".
assert( str);
if (!win || !str)
return ERR;

Expand Down Expand Up @@ -162,6 +165,8 @@ int waddnwstr(WINDOW *win, const wchar_t *wstr, int n)

PDC_LOG(("waddnwstr() - called\n"));

assert( win);
assert( wstr);
if (!win || !wstr)
return ERR;

Expand Down
9 changes: 9 additions & 0 deletions pdcurses/attr.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -133,6 +134,7 @@ int wattroff(WINDOW *win, chtype attrs)
{
PDC_LOG(("wattroff() - called\n"));

assert( win);
if (!win)
return ERR;

Expand All @@ -154,6 +156,7 @@ int wattron(WINDOW *win, chtype attrs)

PDC_LOG(("wattron() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -183,6 +186,7 @@ int wattrset(WINDOW *win, chtype attrs)
{
PDC_LOG(("wattrset() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -228,13 +232,15 @@ int wstandout(WINDOW *win)

chtype getattrs(WINDOW *win)
{
assert( win);
return win ? win->_attrs : 0;
}

int wcolor_set(WINDOW *win, short color_pair, void *opts)
{
PDC_LOG(("wcolor_set() - called\n"));

assert( win);
if (!win)
return ERR;

Expand All @@ -254,6 +260,7 @@ int wattr_get(WINDOW *win, attr_t *attrs, short *color_pair, void *opts)
{
PDC_LOG(("wattr_get() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -305,6 +312,7 @@ int wattr_set(WINDOW *win, attr_t attrs, short color_pair, void *opts)
{
PDC_LOG(("wattr_set() - called\n"));

assert( win);
if (!win)
return ERR;

Expand All @@ -327,6 +335,7 @@ int wchgat(WINDOW *win, int n, attr_t attr, short color, const void *opts)

PDC_LOG(("wchgat() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down
1 change: 1 addition & 0 deletions pdcurses/beep.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ int flash(void)

PDC_LOG(("flash() - called\n"));

assert( curscr);
if (!curscr)
return ERR;

Expand Down
6 changes: 6 additions & 0 deletions pdcurses/bkgd.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -72,6 +73,7 @@ int wbkgd(WINDOW *win, chtype ch)

PDC_LOG(("wbkgd() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -172,6 +174,7 @@ chtype getbkgd(WINDOW *win)
{
PDC_LOG(("getbkgd() - called\n"));

assert( win);
return win ? win->_bkgd : (chtype)ERR;
}

Expand All @@ -180,6 +183,7 @@ int wbkgrnd(WINDOW *win, const cchar_t *wch)
{
PDC_LOG(("wbkgrnd() - called\n"));

assert( wch);
return wch ? wbkgd(win, *wch) : ERR;
}

Expand Down Expand Up @@ -209,6 +213,8 @@ int wgetbkgrnd(WINDOW *win, cchar_t *wch)
{
PDC_LOG(("wgetbkgrnd() - called\n"));

assert( win);
assert( wch);
if (!win || !wch)
return ERR;

Expand Down
6 changes: 6 additions & 0 deletions pdcurses/border.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -141,6 +142,7 @@ int wborder(WINDOW *win, chtype ls, chtype rs, chtype ts, chtype bs,

PDC_LOG(("wborder() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -206,6 +208,7 @@ int whline(WINDOW *win, chtype ch, int n)

PDC_LOG(("whline() - called\n"));

assert( win);
if (!win || n < 1)
return ERR;

Expand Down Expand Up @@ -263,6 +266,7 @@ int wvline(WINDOW *win, chtype ch, int n)

PDC_LOG(("wvline() - called\n"));

assert( win);
if (!win || n < 1)
return ERR;

Expand Down Expand Up @@ -348,6 +352,7 @@ int whline_set(WINDOW *win, const cchar_t *wch, int n)
{
PDC_LOG(("whline_set() - called\n"));

assert( wch);
return wch ? whline(win, *wch, n) : ERR;
}

Expand Down Expand Up @@ -382,6 +387,7 @@ int wvline_set(WINDOW *win, const cchar_t *wch, int n)
{
PDC_LOG(("wvline_set() - called\n"));

assert( wch);
return wch ? wvline(win, *wch, n) : ERR;
}

Expand Down
4 changes: 4 additions & 0 deletions pdcurses/clear.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -58,6 +59,7 @@ int wclrtoeol(WINDOW *win)
PDC_LOG(("wclrtoeol() - called: Row: %d Col: %d\n",
win->_cury, win->_curx));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -93,6 +95,7 @@ int wclrtobot(WINDOW *win)

PDC_LOG(("wclrtobot() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -144,6 +147,7 @@ int wclear(WINDOW *win)
{
PDC_LOG(("wclear() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down
2 changes: 2 additions & 0 deletions pdcurses/delch.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -44,6 +45,7 @@ int wdelch(WINDOW *win)

PDC_LOG(("wdelch() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down
4 changes: 4 additions & 0 deletions pdcurses/deleteln.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -60,6 +61,7 @@ int wdeleteln(WINDOW *win)

PDC_LOG(("wdeleteln() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -122,6 +124,7 @@ int winsdelln(WINDOW *win, int n)

PDC_LOG(("winsdelln() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down Expand Up @@ -156,6 +159,7 @@ int winsertln(WINDOW *win)

PDC_LOG(("winsertln() - called\n"));

assert( win);
if (!win)
return ERR;

Expand Down
9 changes: 9 additions & 0 deletions pdcurses/getyx.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* PDCurses */

#include <curspriv.h>
#include <assert.h>

/*man-start**************************************************************
Expand Down Expand Up @@ -76,55 +77,63 @@ int getbegy(WINDOW *win)
{
PDC_LOG(("getbegy() - called\n"));

assert( win);
return win ? win->_begy : ERR;
}

int getbegx(WINDOW *win)
{
PDC_LOG(("getbegx() - called\n"));

assert( win);
return win ? win->_begx : ERR;
}

int getcury(WINDOW *win)
{
PDC_LOG(("getcury() - called\n"));

assert( win);
return win ? win->_cury : ERR;
}

int getcurx(WINDOW *win)
{
PDC_LOG(("getcurx() - called\n"));

assert( win);
return win ? win->_curx : ERR;
}

int getpary(WINDOW *win)
{
PDC_LOG(("getpary() - called\n"));

assert( win);
return win ? win->_pary : ERR;
}

int getparx(WINDOW *win)
{
PDC_LOG(("getparx() - called\n"));

assert( win);
return win ? win->_parx : ERR;
}

int getmaxy(WINDOW *win)
{
PDC_LOG(("getmaxy() - called\n"));

assert( win);
return win ? win->_maxy : ERR;
}

int getmaxx(WINDOW *win)
{
PDC_LOG(("getmaxx() - called\n"));

assert( win);
return win ? win->_maxx : ERR;
}

Expand Down
Loading

0 comments on commit 96f4984

Please sign in to comment.