Browsing through history in Editbox

Hi all,

I am trying to implent this into an editbox, I am totally lost after editing the source several times, any feedback or info is much appreciated. I hope someone can point me to the right direction..

I have got this so far, I uploaded my whole MS VC++ 6.0 project, if that is no good I can post snippets instead..

Source Link

How about zipping? I can't open rar files. (And I'm sure some others can't either.)

Zip.

Some things I noticed:

You clear the edit before getting it:

Code:

---

char *buf;
int buflen = GetWindowTextLength(hWnd);
if(buflen == 0) return FALSE;
SetDlgItemText(GetParent(hWnd), IDC_COMMAND, "");
buf = malloc(sizeof(char)*++buflen);
GetDlgItemText(GetParent(hWnd), IDC_COMMAND, buf, buflen);
// Should be here

---

And, you have:

Code:

---

Commands[ComIndex] = buf;
/* ComLast = (ComIndex == 0) ? (15) : (ComIndex - 1); */
free(buf);
return TRUE;

---

But char* type is a pointer, so freeing buf after assignment is making Commands[] pointer invalid. So, either, you don't want to free buf, Or you should assign Commands[] with strcpy(). Regardless, you need to free the whole Commands[] array when you close the program.

Those are just some things that I noticed, so there could me more.

Your also assuming that memory allocation is going to work 100% of the time, (malloc returns NULL on failure).
IE

Code:

---

if((buf = malloc(size)) == NULL)
{
    /* malloc failed, do whatever */
}

---

However, I would simply recommend limiting the length of the edit box and then store 'buf' on the stack.

Code:

---

#define MAXBUF 255
/* ... */
char buf[MAXBUF];

---

Also I'm pretty sure GetWindowTextLength doesn't return the how big the value of an edit box is in bytes but rather the 'visable' bytes. (See MSDN).

Alright, I changed to what you guys said, I think only the looping trough commands (via VK_UP and VK_DOWN) should be fixed... The ComIndex should be increased when VK_UP is pressed and decreased when VK_DOWN is pressed, am I right?

I edited your code with notes added (untested).

Code:

---

char *Commands[16] = { NULL };
/* ComIndex is our storage index, ComLast is our retrieval index. */
int ComIndex = 0, ComLast = 0;

LRESULT CALLBACK EditBox(HWND hWnd, UINT Message, WPARAM wParam, LPARAM lParam)
{
        if(Message == WM_KEYDOWN)
        {
                if(wParam == VK_RETURN) {
                       
                        char *buf;

                        int buflen = GetWindowTextLength(hWnd);

                        if (buflen == 0) return FALSE;

                        buf = malloc(sizeof(char) * ++buflen);
                        /* Note: Should check result of malloc here. */

                        /* Note: We have to get the text before we delete it.
                        *      Could use GetWindowText instead. */
                        GetDlgItemText(GetParent(hWnd), IDC_COMMAND, buf, buflen);

                        /* Note: Could be easier to use SetWindowText. */
                        SetDlgItemText(GetParent(hWnd), IDC_COMMAND, "");

                        /* Free the old memory pointer if it exists. */
                        if (Commands[ComIndex]) free(Command[ComIndex]);

                        /* Store the buf memory pointer in the Command array. */
                        Commands[ComIndex] = buf;

                        /* Set retrieval slot to last stored item. */
                        ComLast = ComIndex;

                        /* Move storage slot to next slot in array. */
                        ComIndex++;

                        /* Loop to start if at end of array. */
                        if (ComIndex == 16) ComIndex = 0;

                        return TRUE;
                }
                else if(wParam == VK_UP) {
                       
                        if(!Commands[ComLast]) {
                                /* No item available! */
                                return FALSE;
                        }
                        else {
                                SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
                        }

                        /* Move the retrieval index up one. */
                        ComLast = ComLast - 1;

                        /* Loop to end if required. */
                        if (ComLast == -1) ComLast = 15;
                }

---

I am having trouble with the down arrow button, shouldnt it look like this:

Code:

---

                else if(wParam == VK_DOWN) {
                        if(!Commands[ComLast]) {
                                /* No item available! */
                                return FALSE;
                        }
                        else {
                                SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[++ComLast]);
                        }

                        if (ComLast == 16) ComLast = 0;
                }

---

Quote:

---

Originally Posted by Queatrix

How about zipping? I can't open rar files. (And I'm sure some others can't either.)

---

why should we go through the extra effort to please you if you are too lazy to download WinRar. Its way better than 'ZIP', and its been the standard now for over 10 years.

Chill out. At least I helped, all you did was clutter this thread with a useless/helpless complaint.

I dont really mind putting them into a zip and uploading, anyways to get back on topic :) this is what I got so far, it works okay but there are a few problems.

It browses okay trough the buffer, however when you press the up arrow button then you will have to press the down arrow button twice to display the next command, and vice versa with the up arrow button, when you press the down arrow key, you will have to press twice the upper arrow to get it display the right buffer.

I also noticed that it is crashing at a point when you browse trough the buffer for a while, I think I am forgetting some part, anyone have an idea?

Code:

---

                if(wParam == VK_UP) {
                       
                        if(!Commands[ComLast]) {
                                /* No item available! */
                                SetWindowText(hWnd, "");
                                return FALSE;
                        }
                        else {
                                SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
                        }

                        /* Move the retrieval index up one. */
                        ComLast = ComLast - 1;

                        /* Loop to end if required. */
                        if (ComLast == -1) ComLast = 15;
                }
                else if(wParam == VK_DOWN) {
                        if(!Commands[++ComLast]) { /* Increase here ? */
                                /* SetWindowText(hWnd, ""); */
                                /* No item available! */
                                return FALSE;
                        }
                        else {
                                SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
                        }

                        if (ComLast == 16) ComLast = 0;
                }

---

This is the VK_RETURN part:

Code:

---

                if(wParam == VK_RETURN) {
                       
                        char *buf;

                        int buflen = GetWindowTextLength(hWnd);

                        if (buflen == 0) return FALSE;

                        if((buf = malloc(sizeof(char) * ++buflen)) == NULL) {
                                /* No Memory Space */
                                return FALSE;
                        }

                        /* Note: We have to get the text before we delete it. */
                        GetWindowText(hWnd, buf, buflen);

                        SetWindowText(hWnd, "");

                        /* Free the old memory pointer if it exists. */
                        if (Commands[ComIndex]) free(Commands[ComIndex]);

                        /* Store the buf memory pointer in the Command array. */
                        Commands[ComIndex] = buf;

                        /* Set retrieval slot to last stored item. */
                        ComLast = ComIndex;

                        /* Move storage slot to next slot in array. */
                        ComIndex++;

                        /* Loop to start if at end of array. */
                        if (ComIndex == 16) ComIndex = 0;

                        return TRUE;
                }

---

if(!Commands[++ComLast])

isn't this prefix operation? so you increase firstly the index and then retreive the pointer - probably going out of the array bounds?

And what about wrapping around on the actual number of entered commands - and not on hardcoded 15?

Quote:

---

Originally Posted by vart

probably going out of the array bounds

---

Okay, this should fix the crash, but still, there are some problems with browsing..

Code:

---

                else if(wParam == VK_DOWN) {
                        if (++ComLast == 16) ComLast = 0; /* Check if it has reached 16 */

                        if(!Commands[ComLast]) {
                                /* No item available! */
                                /* SetWindowText(hWnd, ""); */
                                return FALSE;
                        }
                        else {
                                SetWindowText(hWnd, Commands[ComLast]);
                        }
                }

---