Opened 4 years ago

Last modified 3 years ago

#12620 assigned enhancement

Use a map for finding areas in ServerMemoryAllocator

Reported by: Anarchos Owned by: nobody
Priority: low Milestone: Unscheduled
Component: Kits/Application Kit Version: R1/Development
Keywords: ServerMemoryAllocator Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

This is a patch to solve a TODO in src/kits/app/ServerMemoryAllocator.cpp I couldn't test it, but i post it for review here. Don't hesitate to comment !!

Attachments (1)

0006-remplacement-BList-par-TODO.patch (7.0 KB ) - added by Anarchos 4 years ago.
Solving a TODO in ServerMemoryAllocator (UNTESTED)

Download all attachments as: .zip

Change History (7)

by Anarchos, 4 years ago

Solving a TODO in ServerMemoryAllocator (UNTESTED)

comment:1 by Anarchos, 4 years ago

Has a Patch: set

comment:2 by pulkomandy, 4 years ago

Summary: Patch to solve a TODOUse a map for finding areas in ServerMemoryAllocator

comment:3 by axeld, 4 years ago

Thanks for you contribution. A couple of quick remarks:

  • The coding style is questionable. Please have a look at our coding style guidelines (for example, naming of variables). Trying not to make the code stick out would be a good start.
  • Your spacing seems to be off -- we use tabs for indentation, not spaces. The spacing in the original file is okay.
  • Don't leave commented out code in your patches.
  • Please use a typedef for the map.
  • What is "!ret_insert.second" supposed to do?
  • Instead of std::pair, a dedicated structure would make the code a lot more readable.
  • Areas don't seem to be deleted anymore on destruction.
  • What's the point in submitting code you didn't even test?

Most issues make your patch very hard to read and evaluate.

comment:4 by pulkomandy, 3 years ago

Has a Patch: unset

comment:5 by pulkomandy, 3 years ago

Has a Patch: set

Marking the patch as "obsolete", as pointed out by axel above, more work is required.

comment:6 by axeld, 3 years ago

Owner: changed from axeld to nobody
Status: newassigned
Note: See TracTickets for help on using tickets.