Puppy Linux Discussion Forum Forum Index Puppy Linux Discussion Forum
Puppy HOME page : puppylinux.com
"THE" alternative forum : puppylinux.info
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

The time now is Sat 20 Oct 2018, 11:11
All times are UTC - 4
 Forum index » Off-Topic Area » Programming
locking own thread
Post new topic   Reply to topic View previous topic :: View next topic
Page 3 of 3 [36 Posts]   Goto page: Previous 1, 2, 3
Author Message
fredx181


Joined: 11 Dec 2013
Posts: 3481
Location: holland

PostPosted: Fri 08 Jun 2018, 08:19    Post subject:  

Sailor Enceladus wrote:
We could just delete every thread in Puppy Projects that does not fit with the idea mavrothal proposed. Better luck next time guys, should have posted your fork in Puppy Derivatives Twisted Evil Very Happy


Indeed that was an awkward thing to say, probably you may think of it as "Just a joke", well... not funny at all.

Fred

_________________
Dog Linux website
Back to top
View user's profile Send private message 
Sailor Enceladus

Joined: 22 Feb 2016
Posts: 1546

PostPosted: Fri 08 Jun 2018, 08:52    Post subject:  

fredx181 wrote:
Indeed that was an awkward thing to say, probably you may think of it as "Just a joke", well... not funny at all.

I guess we'll never know what mavrothal thinks about it, he seems to be more interested in global warming now Razz
Back to top
View user's profile Send private message 
technosaurus


Joined: 18 May 2008
Posts: 4841
Location: Blue Springs, MO

PostPosted: Fri 08 Jun 2018, 14:20    Post subject: Re: wiak.c for people who love C pointers  

wiak wrote:

Code:
/* **************************************************************************
 Program: wiak (an IPC, non-interactive, shell)  Version: 2.3.0         
//...
/* end_of_func_receive_msgqv */

I am assuming you wanted feedback on the code.
I don't know what is in your header file, but its not portable C. write() is in <unistd.h>
You use sprintf instead of snprintf on a fixed size buffer.
When you do a switch-case, it helps readability if you do it in ASCII order and it can help the compiler optimize.
There are several places where you check the same conditional multiple times - those could be factored out. for example:
Code:
else if ((ptr_wargv_st->wiak_channel[0] != '\0') &&  ...

There are places where you are copying strings (unnecessarily?) instead of just using the pointers, though I didn't fully explore its usage.
One of the switch statements has a lot of repetitive code that could be refactored using fallthroughs (and probably replacing the sprintf with string functions)
Using the *printf functions adds quite a bit of bloat, it looks like a few extra functions could save you several Kb in static builds (not a big concern though)
For readability, try to wrap to 78 characters if possible.
It may still be difficult to read because some functions exceed a page. Consider using inline functions or possibly "hugging your if-elses" so that its possible to read a whole function at a time.
Code:
if (cond){ //comment for cond
  foo();
  return 0;
}else{ //comment for !cond
  bar();
  return 1;
}

vs
Code:
if (cond) //comment for cond
{
  foo();
  return 0;
}
else //comment for !cond
{
  bar();
  return 1;
}

Geany's (and other IDE's) code folding makes the former more condensed yet readable

_________________
Check out my github repositories. I may eventually get around to updating my blogspot.
Back to top
View user's profile Send private message Visit poster's website 
wiak

Joined: 11 Dec 2007
Posts: 957
Location: not Bulgaria

PostPosted: Fri 08 Jun 2018, 14:47    Post subject: Re: wiak.c for people who love C pointers  

technosaurus wrote:
wiak wrote:

Code:
/* **************************************************************************
 Program: wiak (an IPC, non-interactive, shell)  Version: 2.3.0         
//...
/* end_of_func_receive_msgqv */

I am assuming you wanted feedback on the code.


Yes, thanks technosaurus, the codes over 10 years old and was written just off the cuff in a hurry at the time and I did know it was badly needing refactored. Actually that much I remember - I just basically verbatim copied some of the code lines repetively knowing I should factor them later as major simplifications, but never bothered. Also, yeah I wasn't being careful to avoid sprintf's and so on. But yeah, I'll take a quick look at some of your suggestions - I actually can't remember much of what the code does exactly, but that partly why I dug it out because I wanted to remind myself of the IPC details. I would have to look more carefully at it myself to see the string copies I was making directly other than via pointers - that surprises me, i would usually use pointers for that certainly, but you'd have to indeed also look at the header file, because its impossible to tell if some of these variables are pointers or not, but you may well be correct.

Of course, I expected the code to get put over the coals in terms of style and various inefficiencies - back then I was used to writing quick and dirty code without much thought as to how perfect or otherwise it was - main thing was writing quick TCP/IP test scripts (on other scripts in the Uni research group I worked) where code error trapping was irrelevant and time was short so as long as it generated the appropriate protocol data that was good enough. I wasn't a big system programmer where critical details matter - C was just a quick hack tool amongst many others in that early Internet protocol design research group I was in (but that was around 1990 and wiak was first C I had written for almost 20 years is my only excuse), and for me, still is.

You may be doing more regular bigger system C programming than I ever did, and never now will. but at least I know a little, which has been useful to me.

Thanks for your analysis.

wiak
Back to top
View user's profile Send private message 
wiak

Joined: 11 Dec 2007
Posts: 957
Location: not Bulgaria

PostPosted: Fri 08 Jun 2018, 14:53    Post subject:  

This was the header, by the way - I haven't looked at it, and maybe won't either...!

Code:
#include <ctype.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <limits.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <getopt.h>
#include <sys/ipc.h>
#include <sys/msg.h>

#define WIAKFIFOPATH "/tmp/wiak/"
#define FIFOPATHPERMS 0666
#define MBOXPERMS 0666
#define FIFO_DFLT_NAME "wfifo" // default fifo name
#define MSGQV_DFLT_NAMEKEY 45678
#define WAITFIFO 5

#define BUF65536_SIZE 65536
#define BUF32768_SIZE 32768
#define BUF16384_SIZE 16384
#define BUF8192_SIZE 8192
#define BUF4096_SIZE 4096
#define BUF0256_SIZE 256
#define BUF0128_SIZE 128
#define BUF0064_SIZE 64
#define BUF0032_SIZE 32
#define BUF0016_SIZE 16
#define BUF0008_SIZE 8
#define BUF0004_SIZE 4

#define MAX_FIFOMSG BUF65536_SIZE
#define TRUE 0
#define FALSE 1

// The following #defines are RESERVED and subject to change:
#define CHARSTRDATA 1     // Range 1 to 2047.
#define CHARDATA 2048     // planned Range 2048 to 4095 RESERVED
#define LONGINTDATA 4096  // planned Range 4096 to 6143 RESERVED
#define DOUBLEDATA 6144   // planned Range 6144 to 8191 RESERVED
#define STRUCTDATA 8192   // Experimental, uses 8192 and up RESERVED

static key_t msgqv_namekey = MSGQV_DFLT_NAMEKEY; // key (like a numeric name) to use for the msgqv
static char fifo_name[PATH_MAX] = {'\0'}; // complete fifo name
static char wiak_tmp_path[PATH_MAX] = {WIAKFIFOPATH}; // default fifo path
char *getenv_ptr;

// currently only using CHARSTRDATA
// Any code for STRUCTDATA messages is purely experimental
// Currently can --send messages of max sizes =
// 65536 bytes for fifo IPC and 8192 for SysV message queue IPC
// The above values are system/platform dependent. Results are from
// a Linux system with kernel version 2.6.21.5

struct mbox_char { // RESERVED and subject to change
    long int msgtype; // msg queue C-char type.
                      // Initialise to CHARDATA
    char c_data;
};

struct mbox_long { // RESERVED and subject to change
    long int msgtype; // msg queue C-char type.
                      // Initialise to LONGINTDATA
    long int c_data;
};

struct mbox_char_str {
    long int msgtype; // msg queue C-char_stringdata type.
                      // Initialise to CHARSTRDATA
    char c_data[MAX_FIFOMSG]; // to/from mbox channel to/from wargv_st->wiak_send buffer
};

struct mbox_double { // RESERVED and subject to change
    long int msgtype; // msg queue C-char type.
                      // Initialise to DOUBLEDATA
    double c_data;
};

struct msgdata_struct { // used in fifo and SysV msgqueue mode
    struct mbox_char msg_char; // RESERVED and subject to change
    struct mbox_long msg_long; // RESERVED and subject to change
    struct mbox_char_str msg_char_str; // used in fifo and SysV msgqueue mode
    struct mbox_double msg_double; // RESERVED and subject to change
       
    pid_t pid_t_array[BUF0004_SIZE]; // RESERVED and subject to change
};

struct mbox_all {
    long int msgtype; // msg queue C_struct data type.
                      // Initialise to STRUCTDATA
    struct msgdata_struct msgdata;
};

struct wargv {
   char wiak_up[BUF0256_SIZE];   // program <path>/name of wiakapp backend server to raise
   char wiak_mode[BUF0016_SIZE]; // default is fifo (alternatives: msgqv, ... others coming)
   char wiak_send;       // set to FALSE (=1) for no --send option selected (i.e. nothing to send)
                        // and TRUE (=0) for --send option selected (i.e. something to send)
   char wiak_receive[BUF0256_SIZE];  // device on which wiak should receive mbox message
                                     // currently, user should specify stdout
   char wiak_channel[BUF0256_SIZE];  // alphanumeric name for the mbox channel (optional)
                                      // else fifo uses default ch name [+ any zid value]
   pid_t wiak_zid;  // numeric id for identifying the mbox channel (optional)
   char wiak_file[BUF0256_SIZE];    // RESERVED and subject to change
   char wiak_dir[BUF0256_SIZE];    // RESERVED and subject to change
   long int wiak_type; // type/priority of messages to send (used in mess queue mode)
                        // default is CHARSTRDATA = char_string_c_data.
                        // Can use for prioritising SysV message queue data.
                        // struct_c_data messages (STRUCTDATA) are currently experimental
                        // and subject to changes in format
   long int wiak_length; // negative values directly indicate string length to --send
                          // positive values indicate terminating chars to --send
                          // 0 = don't append anything; 1 = '\n' only; 2 = '\n' then a '\0';
                          // 3 = '\0' only (i.e. a simple C char string);
                          // 4 = '\t' only; 5 = '\t' then a '\0';
                          //  6 = '\t', then a '\n'; 7 = '\t', then a '\n' and then a '\0';
                          // default is 1, which results in the same behaviour as the default 
                          // (no options used) bash echo command
   int wiak_perms;       // message box (octal rwx) permissions (used in mess queue mode)
   char wiak_init;         // set to FALSE (=1) for don't make new channel (e.g. fifo)
                          // and TRUE (=0) for make new channel (e.g. create new fifo)
   char wiak_quit;       // set to FALSE (=1) for don't clean up
                        // and TRUE (=0) for clean up e.g. remove fifo(s) or SysV message queue(s)
}wargv_st =
{ //defaults
    "",
    "",
    FALSE,
    "",
    "",
    0,
    "wiakrecord.conf",
    "/usr/local/wiakapps/morfi/",
    CHARSTRDATA,
    1,
    MBOXPERMS,
    FALSE,
    FALSE
};


Yes, all a bit quick and nasty generally. Worked very reliably though (was very efficient in practical use) - that's one thing... Smile

I remember very well the struggle I had writing it - long hours of reminding myself of enough C to cope. After such a long time I was really very rusty. I'm far worse now unfortunately... But I'm retired and an old man now really as far as coding goes so it doesn't matter any more (and even when I wrote wiak.c I was far past middle-aged so not at my sharpest probably). Time you young guys did all the coding!
Back to top
View user's profile Send private message 
wiak

Joined: 11 Dec 2007
Posts: 957
Location: not Bulgaria

PostPosted: Mon 11 Jun 2018, 04:04    Post subject: reconsidering my old wiak.c code
Subject description: in terms of its 'perfection' or otherwise!!! ;-)
 

@technosaurus:

I've now had time to have a quick look at my old program code wiak.c and check it against your criticism.

I don't follow what you mean when you say that it is not 'portable C'. From my later posted wiakapps.h header file you should see that I use unistd.h for POSIX API and write() system call does indeed require that.

Yes, I have used sprintf (carefully) when I could have possibly more safely used snprintf to reduce the chance of buffer overloads.

Now that I've looked I can't say I see what you mean that I copied some strings ("unnecessarily?") when I could just have used pointers. The target and destination arguments required by strcpy() are of type pointer. Certainly, I could have written my own very simple strcpy() function but I am not inclined to do that preferring the often more sophisticated library provided well tested versions. I'm not trying to make tiny C code library substitutes in that sense, which I personally wouldn't trust, no matter how good the coder thinks he or she is. Depends on how critical the code is however in terms of end user purpose.

As I said, in practice, the program 'wiak' did everything asked of it and with no exhibited bugs during my testing; I couldn't ask more than that.

As for readability, I personally hate 'hugged' if..else type formatting and I'm not at all keen on in-line functions either. Regarding the length of my functions, they are not in any way too long for my own ability to comprehend them, which is all I strive for. Readability is IMO largely a matter of taste and more important is sticking to a consistent format.

Regarding the switch case statement - yes, I didn't bother rearranging it in alphabetical order - I don't agree that any compiler optimisation would be anything but negligible if I did. I could have allowed some of the logic to 'fallthrough' as you suggest, but I am very aware that there are dangers involved in allowing fallthroughs to occur - an error in that flowthrough logic can cause a major bug in the resultant code, so generally I much prefer to maintain tight control over each case via specific break statements on each satisfied case condition.

Useful to hear another's perspective though, so thanks again.

wiak

Note: My own self-criticism would actually be about the header file itself, which is a complete mess (I don't know what I was thinking about when I created it). No way should I have been defining static elements in there (when in practice I was using them as globals anyway). Nor should I really have been defining (rather than just declaring anything in there). I only 'got away with it' because there was only one .c source file it was being used in. And as for putting the function declarations in wiak.c rather than inside the header?!!! Of course if I had stared modularising things a bit, I would have had to organise the header (sensibly/correctly) accordingly - but I was too busy testing the IPC operations rather than worrying about (highly) potential linking issues, that didn't arise in this case of single .c source.
Back to top
View user's profile Send private message 
Display posts from previous:   Sort by:   
Page 3 of 3 [36 Posts]   Goto page: Previous 1, 2, 3
Post new topic   Reply to topic View previous topic :: View next topic
 Forum index » Off-Topic Area » Programming
Jump to:  

You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You can download files in this forum


Powered by phpBB © 2001, 2005 phpBB Group
[ Time: 0.0642s ][ Queries: 11 (0.0100s) ][ GZIP on ]