CUDALucas (a.k.a. MaclucasFFTW/CUDA 2.3/CUFFTW)

[flashjh]

Originally Posted by Dubslow

Sometimes Windows will detect programs going into infinite loops, and report that the program is not responding. That's not what happened?

Here's the relevant code:
...
I can't create Windows executables, and I don't know much about MSVS; you'll have to tell flash how to compile it with debugging symbols in Windows.

Edit: Flash! The fix to open_s() never made it into r35! (It should return file_handle, not 0!)

Yes, I see. I'll fix and repost everything in the morning.

EDIT: When quoting the immediately previous message, including the whole thing is unnecessary and only makes thread hard to read. SB

[LaurV]

Version 2.04 beta, standard (this means I did not get yet any fix you are talking here about):
For me it is not really a crash, but it fails to delete the lck file. I used to delete them by hand, as a "routine clearing" process (I have multiple cudalucas folders, for each instance I run, I do all reports by hand and clear the folders after that, by deleting all checkpoints and files except the exe, ini, and worktodo in each folder, this is done by a batch file outside of the folder, in the parent). I will try to let it there to see what it will happen.

[Dubslow]

Originally Posted by LaurV

Version 2.04 beta, standard (this means I did not get yet any fix you are talking here about):
For me it is not really a crash, but it fails to delete the lck file. I used to delete them by hand, as a "routine clearing" process (I have multiple cudalucas folders, for each instance I run, I do all reports by hand and clear the folders after that, by deleting all checkpoints and files except the exe, ini, and worktodo in each folder, this is done by a batch file outside of the folder, in the parent). I will try to let it there to see what it will happen.

You don't get any error messages either, like "Failed to close lockfile" in the code?

And it doesn't crash, but just fails to delete the lock file?

[Batalov]

Regardless of what you are debugging, this line is up to no good:

      if( AID[0] && strncasecmp(AID, "N/A", 3) ) { // If (AID is not null), AND (AID is NOT "N/A") (case insensitive)

[flashjh]

Originally Posted by flashjh

EDIT: When quoting the immediately previous message, including the whole thing is unnecessary and only makes thread hard to read. SB

I understand, but when posting from a mobile phone, it's virtually impossible to edit previous material - the site is not moibile device friendly. As such, when the posts are long, I won't quote anymore from my phone.

Yes, I see. I'll fix and repost everything in the morning.

The mentioned fix is posted.

[Dubslow]

This is from my phone, which is why the PS doesn't quote the mentioned post

Originally Posted by Batalov

Regardless of what you are debugging, this line is up to no good:

      if( AID[0] && strncasecmp(AID, "N/A", 3) ) { // If (AID is not null), AND (AID is NOT "N/A") (case insensitive)

How so? Do you perhaps mean that AID[0] might not be initialized? While it isn't initialized in the declaration, its first use is in get_next_assignment(), which does this "if (key!=NULL)strcopy(*key, assignment.hex_key, MAX_LINE_LENGTH+1);", where the struct char* is initialized in parse_worktodo_line() to 0: "assignment->hex_key[0] = 0;". Hardly clear I'll be sure to fix that and just initialize to null at declaration time.

I'm not sure what else it could be, except perhaps reverse AID and "N/A"?

[Batalov]

If AID and AID[0] are initialized, then this check is useless because strncasecmp works fine with empty strings, and if AID isn't then you are crashing right away.

Writing a comment that simply repeats the content of the line (and contradicts what is actually written, in this case) is bad.

Thinking that just because it (supposedly) doesn't crash (because today you think that you know exactly what is called and when) it is an acceptable code is worse.

[Dubslow]

But I do need to check if it's empty. And as I explained before, the string is always properly initialized (though this isn't clear in the slightest by looking at the code, I admit, and it will be fixed).

If it is empty, yes strncasecmp will handle it fine, but it will compare empty to "N/A" as unequal. Thus just "if(strncasecmp(AID, "N/A", 3))" will end up taking the branch, which I don't want to do with an empty string. That would create "CUDALucas.... AID:" which is silly. If there's no AID, then don't needlessly print "AID:".

Thus the comment is exactly what the code does. Perhaps that's not great practice, but I did it for anyone who's not off-handedly familiar with strncasecmp, which until this discussion, I wasn't.
Edit: Perhaps I meant "if (AID is not EMPTY)", is that what you meant about incorrect comment?

I think it's acceptable code because it's the simplest way to make the program do what I want it to do without causing errors.

And as far as "you think that you know exactly what is called and when", isn't that the whole point of *procedural* programming?

PS I've committed the changes.

[flashjh]

2.04 Beta x64 binaries updated here

[Batalov]

Originally Posted by Dubslow

Edit: Perhaps I meant "if (AID is not EMPTY)", is that what you meant about incorrect comment?

Yes, ok, "EMPTY" is the word I can live with, -- not null!

...but even with "EMPTY" this piece of code sounds like

  if(2+3==5) {  // if two plus three equals five, then

Comments are not for this. Comments should be like

                               dump_interval = MAX(dump_interval,
                                                   DEFAULT_DUMP_INTERVAL + 1);

                                /* make the dump interval a multiple of
                                   the check interval. If this is not done,
                                   eventually we will perform a check and
                                   then less than three iterations later
                                   will get a dump, which performs another
                                   check. The Lanczos recurrence only
                                   guarantees that check vectors more than
                                   three iterations back will be orthogonal
                                   to the current x, so this will cause 
                                   spurious failures */

                                dump_interval += check_interval -
                                                dump_interval %
                                                check_interval;

and not like

                               dump_interval = MAX(dump_interval,
                                                   DEFAULT_DUMP_INTERVAL + 1);
// make dump_interval at least DEFAULT_DUMP_INTERVAL + 1

                                dump_interval += check_interval -
                                                dump_interval %
                                                check_interval;
// add check_interval - dump_interval modulo check_interval to dump_interval

Comments are for briefly explaining the reasoning -- and not for retelling the code once again in human words.

That line needs no comment, but rather a preceding line

assert(AID);
#or, if you prefer
assert(AID != NULL);

Then your conscience is clean and your function can be called in any fashion. (You never program alone. Someone will call your function after the list was all consumed and possibly freed. Or will put it in some callback. In short, you never know.)

[LaurV]

Originally Posted by Dubslow

You don't get any error messages either, like "Failed to close lockfile" in the code?

And it doesn't crash, but just fails to delete the lock file?

No idea. Right now I have no CL work. I started to TF the 332M range to 70, as per discussion in the gpu272 thread, with all the steam, but I had to stop that one too, as I temporarily need the cards for daily work. Like in the interrupt mode, the higher priority interrupt wins. When this is done I will return to mfaktc and 332M5 (already finished the range to 332M4 inclusive, before interruption) which will be done in few days too. Then back to CL-DC. I'll watch for this error and keep you informed if it pops up (I may be that I missed it in the past, as I said, I used to delete the lock file by hand anytime I saw it and that may be a reason why no error like failed to close/delete files). But now I will watch for it.