Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with cinput #65

Closed
giovannipozzobon opened this issue Sep 9, 2024 · 7 comments · Fixed by #66
Closed

Problem with cinput #65

giovannipozzobon opened this issue Sep 9, 2024 · 7 comments · Fixed by #66
Labels
bug Something isn't working

Comments

@giovannipozzobon
Copy link
Contributor

I’m using the libc65 library with Calypsis. A very simple program that requires input from the user.

#include <stdio.h>
#include "debug_calypis.h"
#include "mega65/conio.h"
 
 unsigned char stringa[13];

void main (){
conioinit();

// Clear screen and go to the first position
clrscr();
gotoxy(0,0);

cinput(stringa, 13, CINPUT_ACCEPT_ALPHA);

}

The program freezes and monitor is starting.

I investigated and solved the problem by adding this control
if (len ==0) return;

I hypothesize that the lcopy function messes up when the string length is zero, at least with the Calypsis compiler. I don't know with others.
If the length is greater than zero there is no problem.

void cputsxy(unsigned char x, unsigned char y, const unsigned char* s)
{
    const unsigned char len = (unsigned char)my_strlen((const char*)s);
    if (len ==0) return;
    const unsigned int offset = (y * (unsigned int)g_curScreenW) + x;
    lcopy((unsigned long)s, SCREEN_RAM_BASE + offset, len);
    lfill(COLOR_RAM_BASE + offset, g_curTextColor, len);
    g_curY = y + ((x + len) / g_curScreenW);
    g_curX = (x + len) % g_curScreenW;
}

The problem is that at MEGA65 hardware level at least, len=0 means 64Kbyte. This explains the problem.
Screenshot 2024-09-08 alle 20 33 28

@mlund
Copy link
Collaborator

mlund commented Sep 10, 2024

I did a quick test with llvm-mos and the problem seem to exist there as well. lcopy() uses DMA copy via the F018 list structure where count number of bytes is defined. Could you try to test lcopy() separately to verify that it does nothing when length is zero?

If lcopy() is OK there, I see two routes:

  1. Submit a PR for cputsxy() with the suggested range check, or
  2. Do nothing but add a comment to cputsxy() that the string must be non-zero. On restricted systems like ours, this might be the best option(?)

@mlund
Copy link
Collaborator

mlund commented Sep 10, 2024

Pushed ab8d787 that modifies documentation.

@giovannipozzobon
Copy link
Contributor Author

I vote for the first choice because inside the c_input function there is this code that put '\0' in the buffer.

    for (i = 0; i < buflen; ++i) {
        buffer[i] = '\0';
    } 

@mlund
Copy link
Collaborator

mlund commented Sep 10, 2024

OK, how about placing the range check just before calling cputsxy(), i.e. at line 735 of cinput(). It is less invasive and wouldn't impose checks on known non-zero strings.

@giovannipozzobon
Copy link
Contributor Author

Do you mean like something that?

        if (strlen(buffer) != 0)  
            cputsxy(sx, sy, buffer);

Ok for me.

@mlund mlund added the bug Something isn't working label Sep 10, 2024
@mlund
Copy link
Collaborator

mlund commented Sep 10, 2024

Do you mean like something that?

        if (strlen(buffer) != 0)  
            cputsxy(sx, sy, buffer);

Ok for me.

Yes, exactly. Would you mind making a pull request?

giovannipozzobon added a commit to giovannipozzobon/mega65-libc that referenced this issue Sep 10, 2024
@giovannipozzobon
Copy link
Contributor Author

Do you mean like something that?

        if (strlen(buffer) != 0)  
            cputsxy(sx, sy, buffer);

Ok for me.

Yes, exactly. Would you mind making a pull request?

OK. Done.
Thanks.

@mlund mlund mentioned this issue Sep 10, 2024
5 tasks
@mlund mlund linked a pull request Sep 10, 2024 that will close this issue
5 tasks
@mlund mlund closed this as completed in #66 Sep 11, 2024
mlund pushed a commit that referenced this issue Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants