C - Argparse (Argument struct)

Question

I'm trying to make something like Python's argparse in C.

I created this Argument struct and functions; is my code good so far?

Argument.h:

#ifndef ARGUMENT_H
#define ARGUMENT_H
#include <ctype.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdbool.h>
struct Argument{
 char **names;
 char *action;
 char *metavar;
 char *nargs;
 char *type;
 char **choices;
 char *defaultValue;
 bool required;
 char *help;
};
struct Argument Argument_new(char**,char*,char*,char*,char*,char**,char*,bool*,char*);
#endif

Argument.c:

#include "argument.h"
const char *Argument_ACTIONS[]={
 "default",
 "store_true",
 "store_false",
 "show_help",
 "show_version",
 NULL
};
const char *Argument_NARGS_PATTERNS[]={
 "([1-9]\\d*)?\\+",
 "[1-9]\\d*",
 "[1-9]\\d*-[1-9]\\d*"
};
struct Argument Argument_new(char **names,char *action,char *metavar,char *nargs,char *type,char **choices,char *defaultValue,bool *required,char *help){
 struct Argument self;
 size_t namesCount=0;
 char **namesStart=names;
 while(*names++){
 namesCount++;
 }
 int i;
 self.names=malloc(namesCount*sizeof(*names));
 for(i=0,names=namesStart;*names;names++,i++){
 if(false/*not match(*names,"^-{1,2}(?!-)")*/){
 //error
 }else{
 self.names[i]=malloc(strlen(*names)+1);
 strcpy(self.names[i],*names);
 }
 }
 if(action==NULL){
 self.action=malloc(strlen(Argument_ACTIONS[0])+1);
 strcpy(self.action,Argument_ACTIONS[0]);
 }else{
 bool good=false;
 const char **ptr=Argument_ACTIONS;
 while(*ptr){
 if(strcmp(action,*ptr)==0){
 good=true;
 break;
 }
 ptr++;
 }
 if(!good){
 //error
 }else{
 self.action=malloc(strlen(action)+1);
 strcpy(self.action,action);
 }
 }
 if(metavar==NULL){
 self.metavar=malloc(strlen(self.names[namesCount-1])+1);
 strcpy(self.metavar,self.names[namesCount-1]);
 char *ptr=self.metavar;
 while(*ptr){
 *ptr=(char)toupper(*ptr);
 ptr++;
 }
 }else{
 self.metavar=malloc(strlen(metavar)+1);
 strcpy(self.metavar,metavar);
 }
 if(nargs==NULL){
 self.nargs=malloc(strlen("1")+1);
 strcpy(self.nargs,"1");
 }else{
 if(false/*not match(*nargs,???)*/){
 //error
 }else{
 self.nargs=malloc(strlen(nargs)+1);
 strcpy(self.nargs,nargs);
 }
 }
 if(type==NULL){
 self.type=malloc(strlen("string")+1);
 strcpy(self.type,"string");
 }else{
 if(false/*not match(*type,"^(string|((\+|-)?int((>|<|>=|<=|!=)-?[1-9]\d+)?))$")*/){
 //error
 }else{
 self.type=malloc(strlen(type)+1);
 strcpy(self.type,type);
 }
 }
 if(choices==NULL){
 self.choices=malloc(sizeof(NULL));
 self.choices=NULL;
 }else{
 size_t choicesCount=0;
 char **choicesStart=choices;
 while(*choices++){
 choicesCount++;
 }
 self.choices=malloc(choicesCount*sizeof(*choices));
 for(i=0,choices=choicesStart;*choices;choices++,i++){
 self.choices[i]=malloc(strlen(*choices)+1);
 strcpy(self.choices[i],*choices);
 }
 }
 if(defaultValue==NULL){
 if(self.choices==NULL){
 self.defaultValue=malloc(sizeof(NULL));
 self.defaultValue=NULL;
 }else{
 self.defaultValue=malloc(strlen(self.choices[0])+1);
 strcpy(self.defaultValue,self.choices[0]);
 }
 }else{
 self.defaultValue=malloc(strlen(defaultValue)+1);
 strcpy(self.defaultValue,defaultValue);
 }
 self.required=malloc(sizeof(bool));
 if(self.action==Argument_ACTIONS[0]){
 if(self.defaultValue!=NULL){
 self.required=false;
 }else if(required!=NULL){
 self.required=required;
 }else{
 self.required=true;
 }
 }else{
 self.required=false;
 }
 if(help==NULL){
 self.help=malloc(sizeof("")+1);
 strcpy(self.help,"");
 }else{
 self.help=malloc(sizeof(strlen(help))+1);
 strcpy(self.help,help);
 }
 return self;
}

I want to know what can be done better and "cleaner".

Answer 1

Includes

The interface needs only <stdbool.h> - the rest of the standard headers only need to be included in the implementation file.

Interface

The declaration isn't very helpful:

struct Argument Argument_new(char**,char*,char*,char*,char*,char**,char*,bool*,char*);

What does each argument mean? What are the valid ranges of each argument? This is where users will look for documentation, but they find nothing.

Memory allocation

Match the allocation to the type we're assigning to, so we don't have to go back and check whether the types match:

self.names = malloc(namesCount * sizeof *self.names);

And we must test whether we got a valid pointer back from malloc()!

Dead code

This block looks incomplete; should it be finished, or removed?

 if(false/*not match(*names,"^-{1,2}(?!-)")*/){
 //error

sizeof or strlen()

This probably isn't what you meant:

 self.help=malloc(sizeof(strlen(help))+1);

sizeof strlen(help) is sizeof (size_t). You probably meant simply

 self.help = malloc(strlen(help)+1);

Don't forget to clean up

At the same time as you write a "constructor" that does lots of malloc(), you should be writing the corresponding clean-up function, that knows how to free() all these allocations.

• DeBos: Thanks for review. I changed the declaration to: struct Argument Argument_new(char **names,char *action,char *metavar,char *nargs,char *type,char **choices,char *defaultValue,bool *required,char *help); so its more helpful and clean than it was. Is there any difference between sizeof *names and sizeof(*names)? "This block looks incomplete; should it be finished, or removed?". I have little problem with regex on windows so I will finish that blocks of code. "This probably isn't what you meant". You are right, I didn't notice that, thanks again.
• Toby Speight: sizeof (*names) is just the same as sizeof *names (the parentheses are just ordinary expression parentheses, but sometimes people think that sizeof needs them; sometimes you also see redundant parens with return. I think they look ugly when they are not needed!
• Martin R: Minor nitpicking: unless I am mistaken, sizeof strlen(help) is sizeof(size_t).
• Toby Speight: Thanks Martin - fixed (I must be showing my age there!)