Skip to main content
Topic: ICC (Read 10855 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

ICC

Reply #50
Quote
Well, the obvious question is whether "m_KeyFocus->GetText()" could be returning NULL.

Assuming that's not the case, then the problem is apparently in search_backwards().  I'll take a look at it. 

I should mention, though, that search_backwards() probably isn't doing what you are expecting.  Search backwards searches from bottom to top, meaning that it should be finding the last entry when you are calling it, rather than the previously found entry.

Yeah, I just looked at your code again and you don't use search backwards for vk_back and yeah, it won't do what I was thinking.

Still, the text ptr. is def. not null, I am stepping it in the debugger and see "bec" being passed.

EDIT

Yeah, changed my code to not call search_backwards and just call search again when the the string is "backspaced" and no crashes.  Works the same as your edit box.

OK, doesn't work the same (my logic is still not right for an incremental find) but no crashes....

/EDIT

ICC

Reply #51
Playlist Find 0.6.b11.013

compiled: 2003.06.07

Quote
[0.6.b11.013]
Fixed:   Bug in interface (wrapping on search_backwards).
Fixed:   Possible (?) crash when searching backwards when no item
  selected.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip

The searching backwards bug should be fixed.  I couldn't duplicate the crash, but I could see where the bug was.

This should perform an incremental search:
while (USER_WANTS_TO_SEARCH)
{
playlist_oper::get()->set_focus_sel(global_playlist_find::get()->search("YOUR SEARCH STRING HERE"));
}

ICC

Reply #52
Quote
Playlist Find 0.6.b11.013

compiled: 2003.06.07

Quote
[0.6.b11.013]
Fixed:   Bug in interface (wrapping on search_backwards).
Fixed:   Possible (?) crash when searching backwards when no item
  selected.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip

The searching backwards bug should be fixed.  I couldn't duplicate the crash, but I could see where the bug was.

This should perform an incremental search:
while (USER_WANTS_TO_SEARCH)
{
playlist_oper::get()->set_focus_sel(global_playlist_find::get()->search("YOUR SEARCH STRING HERE"));
}

Yeah, fixed the crash.

I think search_back is what I want for my algorithm

as the user types in chars I keep calling

search(str)

as soon as they backspace I want to find the previous match and it appears search_back does that.

anyway, seems solid.

Thanks.

ICC

Reply #53
For something like that, you should just store the last index found.

ICC

Reply #54
Quote
Quote
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.

So I would inlcude foo_history.h in my build and then simply enum the history service and use it?

Almost. Include foo_history.h and add foo_history.cpp to your project (it contains the guid stuff). You don't need to enumerate yourself, history::get() does this for you. You should check for a null pointer in case the foo_history plugin is not present.
Sorry for the late answer, I haven't been reading the forum for some days.

ICC

Reply #55
Quote
Quote
Quote
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.

So I would inlcude foo_history.h in my build and then simply enum the history service and use it?

Almost. Include foo_history.h and add foo_history.cpp to your project (it contains the guid stuff). You don't need to enumerate yourself, history::get() does this for you. You should check for a null pointer in case the foo_history plugin is not present.
Sorry for the late answer, I haven't been reading the forum for some days.

No problem on the late reply.  This is also how Curi0us_George did the playlistfind interface (using a global "get()" function) which after fleshing out some problems on both sides is working very well now.  I'm doing a similar NULL pointer check for playlistfind and logging a console::error if the look attempts to use playlistfind w/o the service being available.

I'll check out foo_history next and foo_bookmark when available

ICC

Reply #56
Playlistfind should prob. check the bounds of the start_index parameter because you can still crash playlistfind with bad parameters.

I think the problem is associated with the wrapping code which ends up calling do_matching which crashes if the start index is out of bounds (ie > the get_count() )

Code: [Select]
  // if we get to this point, we didn't find an item that
  // matched after the current one, so we start over from the top
  if (wrap)
  {
   count = start_index;
   for (int i = 0; i < count; i++)
   {
    if (do_matching(str, i)) return i;
   }
  }


Code: [Select]
    bool do_matching(const char * str, int playlist_index) {
 static string8 out;
 out.reset();
 metadb_handle * item;
 item = get_item(playlist_index);
 item->handle_format_title(out,get_search_format(),NULL);
 item->handle_release();

ICC

Reply #57
Well, I'm not checking parameters within the playlist_find class, because I think that's the responsibility of the interface user.  However, I am doing some basic bounds checking in the global_playlist_find class.

Code: [Select]
    int search(const char * str, int start_index, bool wrap)
    {
 if (start_index == -1)
 {
     start_index = g_oper->get_focus() + 1;
     if (start_index > g_oper->get_count())
     {
   start_index = g_oper->get_count();
     }
 }
 return glob_find->search(str,start_index,wrap);
    }


Have you actually crashed it with a normal input parameter?  (i.e. -24 dousn't count as "normal")

ICC

Reply #58
Quote
Well, I'm not checking parameters within the playlist_find class, because I think that's the responsibility of the interface user.  However, I am doing some basic bounds checking in the global_playlist_find class.

Code: [Select]
    int search(const char * str, int start_index, bool wrap)
    {
 if (start_index == -1)
 {
     start_index = g_oper->get_focus() + 1;
     if (start_index > g_oper->get_count())
     {
   start_index = g_oper->get_count();
     }
 }
 return glob_find->search(str,start_index,wrap);
    }


Have you actually crashed it with a normal input parameter?  (i.e. -24 dousn't count as "normal")

No, it works fine as long as you send an index that is in range.  I had an error and sent a bad value which, yes, was my fault.  I'm just saying a simple check would prevent the crash (maybe playlistfind could log an error instead).

ICC

Reply #59
I generally avoid doing much checking, because then everything ends up getting checked on both sides.

ICC

Reply #60
Quote
I generally avoid doing much checking, because then everything ends up getting checked on both sides.

well, like I said, it was a problem on my side so it isn't a big deal I just think returning -1 instead of a crash would be more helpful for someone else who might want to use the foo_playlistfind service.  If I send a bad URL to foo_inet_read I'd expect it to return an error, not crash foobar.

You can also crash it (same bug really) by clearing the playlist in the middle of a playlist find.  While overall I agree with you about double checking, etc.  I think that this particular case would be best served for all potential clients if you just add a simple bounds check.  At the same time its not worth arguing over.

EDIT

Also, if I send out of bounds values to the playlist_oper service they are ignored or error value is returned.

/EDIT

ICC

Reply #61
I might add some checking to the actual search later.  It should really be looking for NULL metadb_handles anyway.

ICC

Reply #62
Quote
I might add some checking to the actual search later.  It should really be looking for NULL metadb_handles anyway.

Thanks.

ICC

Reply #63
Curi0us_George.

When you get a chance to work on the playlistfind component again could you add a get_search_format() accessor to the global interface.  I want to be able to get that so I can format text for display in my component based on the current searching format.

Thanks.


ICC

Reply #65
Quote
Curi0us_George.

When you get a chance to work on the playlistfind component again could you add a get_search_format() accessor to the global interface.  I want to be able to get that so I can format text for display in my component based on the current searching format.

Thanks.

I haven't looked at 0.7 SDK but I heard some mention of public config variables so perhaps this is a non-issue now???

 

ICC

Reply #66
I won't be making that a public variable. 

But the latest version I uploaded already has an accessor method.  I changed the GUID to prevent problems, but it also has a method to retrieve the version number, so I shouldn't need to change the GUID again.

 
SimplePortal 1.0.0 RC1 © 2008-2019