[haiku-commits] Re: r40826 - haiku/trunk/src/apps/screenshot
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sun, 06 Mar 2011 11:16:52 +0100
stpere@xxxxxxxxx wrote:
> + case B_REFS_RECEIVED:
> + {
> + entry_ref ref;
> + if (message->FindRef("refs", &ref) == B_OK) {
> + BEntry entry(&ref, true);
> + BPath path;
> + entry.GetPath(&path);
Error checks! The link you might resolve can be as invalid as the
entry_ref you got in the first place.
> + BString label(path.Path());
> + _AddItemToPathMenu(path.Path(), label, 3, true);
Looks like a bad API decision to use BStrings here.
> @@ -501,6 +514,24 @@
> ScreenshotWindow::_AddItemToPathMenu(const char* path, BString&
> label,
> int32 index, bool markItem)
> {
> + /* Make sure that item won't be a duplicata of an existing one */
Typo, and prefer C++ style comments.
> + for (int32 i = fOutputPathMenu->CountItems(); i > 0; --i) {
> + BMenuItem* menuItem = fOutputPathMenu->ItemAt(i - 1);
> + BMessage* message = menuItem->Message();
> + const char* pathFromItem;
> + if (message != NULL && message->what == kLocationChanged) {
> + if (message->FindString("path", &pathFromItem) == B_OK)
> {
> + if (!strcmp(path, pathFromItem)) {
This desperately cries for the && operator :-)
Bye,
Axel.
Other related posts:
