[haiku-commits] Re: r40317 - haiku/trunk/src/tests/kits/net/preflet/ServicesAddOn
- From: scottmc <scottmc2@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Sat, 29 Jan 2011 13:52:38 -0800
CC'ing kallisti5@xxxxxxxxxxx, just in case he's not already on the
commits list...
On Sat, Jan 29, 2011 at 12:59 PM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:
> kallisti5@xxxxxxxxxxx wrote:
>> Log:
>> added *basic* ability for services addon to parse xinetd, now we can
>> list active network services in the services view
>
> Please, for the name of god, use the driver settings for this.
> When in doubt, it always makes sense to look who else is going to parse
> those (it's the net_server, btw, which also completely replaces the
> need to any inetd).
>
>> + if (ParseInetd() == B_ERROR)
>> + ParseXinetd();
>
> Don't test for specific errors, test against it, ie:
> if (ParseInetd() != B_OK)
> ...
>
>> return new ServicesAddOn(image);
>> }
>>
>> +
>> // #pragma mark -
>> ServicesAddOn::ServicesAddOn(image_id image)
>
> Also two blank lines after the #pragma.
>
>> + BScrollView* sv = new BScrollView( "system_services_scrollview",
>> + fServicesListView,
>> B_FOLLOW_ALL_SIDES,
>> + B_WILL_DRAW | B_FRAME_EVENTS,
>> false, true);
>
> Please indent only a single tab.
>
>> + FILE *f = fopen("/boot/common/settings/network/services", "r");
>> + if (f) {
>> + char line[1024], *l;
>
> Please don't use variable names like 'f' and 'l'. 'i' for an index is
> the only acceptable exception, really.
>
>> + if (line[0] == '#' || line[0] == '\n')
>> + continue;
>> + // Skip commented out lines
>
> Also comments count to the multi-line rule, ie. use {} here.
>
>
>> + return B_OK;
>> + } else
>> + return B_ERROR;
>
> Don't use code like this, either solve the issue where it happens, ie.:
> FILE* file = ...;
> if (file == NULLL)
> return errno;
>
> Or, don't use "else" - it is completely superfluous, and just makes
> parsing a bit harder than needed.
>
> Also, use meaningful error codes when available. Since this is not for
> any user to see, it doesn't really matter, but still nothing to let
> slip in.
>
>> + * Alexander von Gluck, kallisti5@xxxxxxxxxxx
>> + */
>> +
>> +#include "NetworkSetupAddOn.h"
>> +
>> +class ServicesAddOn : public NetworkSetupAddOn {
>
> Coding style: two blank lines for each of the ones above.
>
>> +protected:
>> + BListView* fServicesListView;
>> +public:
>> + ServicesAddOn(image_id addon_image);
>> + BView* CreateView(BRect* bounds);
>> + const char* Name() { return "Services"; };
>> + status_t ParseInetd();
>> + status_t ParseXinetd();
>
> Coding style: proper indentation.
> Please read our style guideline again, and try to follow it.
>
> Also, I actually mention all those things so that you can actually fix
> them, not just because I have nothing better to do.
>
> Bye,
> Axel.
>
>
>
Other related posts:
