On Sat, Mar 21, 2015 at 11:48:58AM +0100, H.G. Muller wrote: > > > Thomas Adam schreef op 3/21/2015 om 12:06 AM: > > > >I see---it seems a little... hmm, odd, that the presense of the dialogue > >is the thing to check. That there is no data in glc seems the more > >technically correct thing to check for. > > > I think the two are always correlated, as XBoard always created the dialog > as soon > as it gets a non-empty game list. I guess at the time it felt natural to > test for this, > because I assumed that the actual segfault was due to accessing a > non-existent widget. > So I added a test for its existence, I could have been wrong about that, > though.
Oh that's fine, it's just a little odd to test for this based on a GUI widget rather than the backend data (which ultimately drives the widget's ability). It's fine the way you have it, although if that situation were to ever change, then the check would also have to change; hence why looking at the data structure gives a little more robustness. But ultimately it's a moot point, at least it's fixed. :) > >It bothers me, because it's crufty, and often that looks bad, especially > >in terms of code structure, etc. > Well, I am not saying that this should not be done. I just explained I am > not very motivated > to do it. I am basically only interested in adding new functionality, and > then usually because > it is functionality I need myself as an XBoard/WinBoard user. So it is all > the better if there > are other people that would be interested in cleaning up the code. (Which > was already in > a pretty sorry state by the time I got involved.) That's understandable. Of course, the motivation from my point of view (which you could argue is something akin to yak-shaving), is that it will make contributing new features slightly easier, if the state of the surrounding code is a little more consistent, if that makes sense? > The FREE and ASSIGN macros are things I added myself relatively recently, > because I got > a bit tired from writing "if(f) free(f); f=dup(x);" all the time. So in new > code I use those. That's OK, their use can always be expanded as the surrounding code is updated, etc. That'll be a good change to adopt them. > But all the old code still writes it explicitly; I did not see any point in > replacing working code, > as thie macros were intended to save me work, not produce it. > It would be good to also have a macro DEBUG for the construct > "if(appData.debugMode") fprintf(debugFP, ", which is also something I have > to type > annoyingly often. There's a few ways of solving this. You could have a log_debug() function or something which does this, perhaps even a runtime toggle? Who knows. > I wasn't even aware something like asprintf existed. I don't know if there > is a particular > reason avoiding it. (Like compatibility with old libraries that do not have > it. Amazingly > enough even snprintf seems to give problems in MicroSoft Visual Studio.) In > code I add > I usually print to declare static or automatic arrays, rather than to > allocated memory. That sort of change is the more useful one, to reduce the malloc/snprintf code-churn. I had a look at this, and portable code can always provide its own implementation if it's not found via configure, etc. I'm prototyping this at the moment. > At one point we tried to replace all strcpy by safeStrCpy. But being > dogmatic about that > often leads to very silly code, like safeStrCpy(*command, 512, "go\n"); > where there is > no doubt at all that the 3 printed characters will fit in the target, and > you have to assume > the size of the target anyway, because it is only available as (char*). Uh huh. I would strongly suggest strlcpy() though. > Well, this is a problem. Anything change in the back-end that would require > a compensating > change in the XBoard front-end are likely to break WinBoard, if the WinBoard > front-end > would not get the compensating change too. A weaker problem is functional > equivalence; > it is undesirable if features added to XBoard are not operational in > WinBoard for lack of > front-end functionality, even though they compile OK. I understand that, and I'd obviously do my best. But without a means of testing this, I'm at a very loose-end indeed over this, and obviously do not wish to break Winboard as a result. The winboard/makefile.gcc file suggests that it can be cross-compiled, yet it's clearly not received any love for sometime, since there's calls to -mno-cygwin which have been deprecated options to GCC---and in more recent versions have been completely removed (hence 'make -f makefile.gcc' now errors out). So although I could install the msys toolchain on my workstation, I am still dubious whether I'll be able to cross-compile at all. Even if that were successful somehow, is this testable under wine or some such, or are such tests misleading? It sounds to me from what you're saying that the coupling between the backend and Winboard might still be a little too tight---that is, how much of the WIN32 code has actually been shifted into winboard itself? Obviously, changes to the backend, if that separation were completely true, ought to reduce the chances of breaking Winboard. I don't like the thought of developing for XBoard, knowing that it has the ability to break Winboard, so anything you can suggest to me to reduce this likelihood is much appreciated. Thanks! -- Thomas Adam
