Constructor for a packagetarget struct
1
$begingroup$
As many of you know goto is usually signs of code smell. However I thought this could be an appropriate case, and would like confirmation or criticism.
Unnecessary section such as the called functions were removed. Every non-standard function returns an int as status, 0 is "OK"; except linkedlist_open() which returns a pointer which could be NULL if the system runs out of memory.
packagetarget_close() does implement the necessary NULL checks.
packagetarget *packagetarget_open()
{
packagetarget *target = (packagetarget*) malloc(sizeof(packagetarget));
if (!target) return NULL;
if (packagetarget_setname(target, ""))
goto disposer;
if (packagetarget_setsys(target, PACKAGETARGET_SYS))
goto disposer;
if (packagetarget_setarch(target, PACKAGETARGET_ARCH))
goto disposer;
if (packagetarget_setmin(target, PACKAGETARGET_MIN))
goto disposer;
if (packagetarget_setver(target, PACKAGETARGET_VER))
goto disposer;
if (packagetarget_setmax(target, PACKAGETARGET_MAX))
goto disposer;
linkedlist *comp = linkedlist_open();
if (!comp) goto disposer;
target->comp = comp;
return target;
disposer:
packagetarget_close(target);
return NULL;
}
Each setter function follows a simple pattern. Since each setter is identical, I will only put the code for packagetarget_setname.
int packagetarget_setname(packagetarget *target, char *name)
{
if (!target) return 1;
if (!name) return 2;
target->name = realloc(target->name, strlen(name) * sizeof(char));
if (!target->name) return 3;
strcpy(target->name, name);
return 0;
}
Here is the packagetarget_close function:
void packagetarget_close(packagetarget *target)
{
if (!target) return;
if (target->name) free(target->name);
if (target->sys) free(target->sys);
if (target->arch) free(target->arch);
if (target->min) free(target->min);
if (target->ver) free(target->ver);
if (target->min) free(target->min);
if (target->comp) linkedlist_close(target->comp);
free(target);
return;
}
linkedlist_open is a similar function to the packagetarget_open.
linkedlist *linkedlist_open()
{
linkedlist *list = (linkedlist*) malloc(sizeof(linkedlist));
if (!list) return NULL;
list->head = NULL;
list->length = 0;
list->remhook = NULL;
return list;
}
The one thing most of these functions is they may fail when the system runs out of memory. So I have implemented checks for each step.
c error-handling memory-management constructor
edited 39 mins ago
200_success
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
|
show 1 more comment
1
$begingroup$
As many of you know goto is usually signs of code smell. However I thought this could be an appropriate case, and would like confirmation or criticism.
Unnecessary section such as the called functions were removed. Every non-standard function returns an int as status, 0 is "OK"; except linkedlist_open() which returns a pointer which could be NULL if the system runs out of memory.
packagetarget_close() does implement the necessary NULL checks.
packagetarget *packagetarget_open()
{
packagetarget *target = (packagetarget*) malloc(sizeof(packagetarget));
if (!target) return NULL;
if (packagetarget_setname(target, ""))
goto disposer;
if (packagetarget_setsys(target, PACKAGETARGET_SYS))
goto disposer;
if (packagetarget_setarch(target, PACKAGETARGET_ARCH))
goto disposer;
if (packagetarget_setmin(target, PACKAGETARGET_MIN))
goto disposer;
if (packagetarget_setver(target, PACKAGETARGET_VER))
goto disposer;
if (packagetarget_setmax(target, PACKAGETARGET_MAX))
goto disposer;
linkedlist *comp = linkedlist_open();
if (!comp) goto disposer;
target->comp = comp;
return target;
disposer:
packagetarget_close(target);
return NULL;
}
Each setter function follows a simple pattern. Since each setter is identical, I will only put the code for packagetarget_setname.
int packagetarget_setname(packagetarget *target, char *name)
{
if (!target) return 1;
if (!name) return 2;
target->name = realloc(target->name, strlen(name) * sizeof(char));
if (!target->name) return 3;
strcpy(target->name, name);
return 0;
}
Here is the packagetarget_close function:
void packagetarget_close(packagetarget *target)
{
if (!target) return;
if (target->name) free(target->name);
if (target->sys) free(target->sys);
if (target->arch) free(target->arch);
if (target->min) free(target->min);
if (target->ver) free(target->ver);
if (target->min) free(target->min);
if (target->comp) linkedlist_close(target->comp);
free(target);
return;
}
linkedlist_open is a similar function to the packagetarget_open.
linkedlist *linkedlist_open()
{
linkedlist *list = (linkedlist*) malloc(sizeof(linkedlist));
if (!list) return NULL;
list->head = NULL;
list->length = 0;
list->remhook = NULL;
return list;
}
The one thing most of these functions is they may fail when the system runs out of memory. So I have implemented checks for each step.
c error-handling memory-management constructor
edited 39 mins ago
200_success
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not withgotoconstructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.
$endgroup$
– Mast
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct namedpackagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago
|
show 1 more comment
1
1
1
$begingroup$
As many of you know goto is usually signs of code smell. However I thought this could be an appropriate case, and would like confirmation or criticism.
Unnecessary section such as the called functions were removed. Every non-standard function returns an int as status, 0 is "OK"; except linkedlist_open() which returns a pointer which could be NULL if the system runs out of memory.
packagetarget_close() does implement the necessary NULL checks.
packagetarget *packagetarget_open()
{
packagetarget *target = (packagetarget*) malloc(sizeof(packagetarget));
if (!target) return NULL;
if (packagetarget_setname(target, ""))
goto disposer;
if (packagetarget_setsys(target, PACKAGETARGET_SYS))
goto disposer;
if (packagetarget_setarch(target, PACKAGETARGET_ARCH))
goto disposer;
if (packagetarget_setmin(target, PACKAGETARGET_MIN))
goto disposer;
if (packagetarget_setver(target, PACKAGETARGET_VER))
goto disposer;
if (packagetarget_setmax(target, PACKAGETARGET_MAX))
goto disposer;
linkedlist *comp = linkedlist_open();
if (!comp) goto disposer;
target->comp = comp;
return target;
disposer:
packagetarget_close(target);
return NULL;
}
Each setter function follows a simple pattern. Since each setter is identical, I will only put the code for packagetarget_setname.
int packagetarget_setname(packagetarget *target, char *name)
{
if (!target) return 1;
if (!name) return 2;
target->name = realloc(target->name, strlen(name) * sizeof(char));
if (!target->name) return 3;
strcpy(target->name, name);
return 0;
}
Here is the packagetarget_close function:
void packagetarget_close(packagetarget *target)
{
if (!target) return;
if (target->name) free(target->name);
if (target->sys) free(target->sys);
if (target->arch) free(target->arch);
if (target->min) free(target->min);
if (target->ver) free(target->ver);
if (target->min) free(target->min);
if (target->comp) linkedlist_close(target->comp);
free(target);
return;
}
linkedlist_open is a similar function to the packagetarget_open.
linkedlist *linkedlist_open()
{
linkedlist *list = (linkedlist*) malloc(sizeof(linkedlist));
if (!list) return NULL;
list->head = NULL;
list->length = 0;
list->remhook = NULL;
return list;
}
The one thing most of these functions is they may fail when the system runs out of memory. So I have implemented checks for each step.
c error-handling memory-management constructor
edited 39 mins ago
200_success
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
As many of you know goto is usually signs of code smell. However I thought this could be an appropriate case, and would like confirmation or criticism.
Unnecessary section such as the called functions were removed. Every non-standard function returns an int as status, 0 is "OK"; except linkedlist_open() which returns a pointer which could be NULL if the system runs out of memory.
packagetarget_close() does implement the necessary NULL checks.
packagetarget *packagetarget_open()
{
packagetarget *target = (packagetarget*) malloc(sizeof(packagetarget));
if (!target) return NULL;
if (packagetarget_setname(target, ""))
goto disposer;
if (packagetarget_setsys(target, PACKAGETARGET_SYS))
goto disposer;
if (packagetarget_setarch(target, PACKAGETARGET_ARCH))
goto disposer;
if (packagetarget_setmin(target, PACKAGETARGET_MIN))
goto disposer;
if (packagetarget_setver(target, PACKAGETARGET_VER))
goto disposer;
if (packagetarget_setmax(target, PACKAGETARGET_MAX))
goto disposer;
linkedlist *comp = linkedlist_open();
if (!comp) goto disposer;
target->comp = comp;
return target;
disposer:
packagetarget_close(target);
return NULL;
}
Each setter function follows a simple pattern. Since each setter is identical, I will only put the code for packagetarget_setname.
int packagetarget_setname(packagetarget *target, char *name)
{
if (!target) return 1;
if (!name) return 2;
target->name = realloc(target->name, strlen(name) * sizeof(char));
if (!target->name) return 3;
strcpy(target->name, name);
return 0;
}
Here is the packagetarget_close function:
void packagetarget_close(packagetarget *target)
{
if (!target) return;
if (target->name) free(target->name);
if (target->sys) free(target->sys);
if (target->arch) free(target->arch);
if (target->min) free(target->min);
if (target->ver) free(target->ver);
if (target->min) free(target->min);
if (target->comp) linkedlist_close(target->comp);
free(target);
return;
}
linkedlist_open is a similar function to the packagetarget_open.
linkedlist *linkedlist_open()
{
linkedlist *list = (linkedlist*) malloc(sizeof(linkedlist));
if (!list) return NULL;
list->head = NULL;
list->length = 0;
list->remhook = NULL;
return list;
}
The one thing most of these functions is they may fail when the system runs out of memory. So I have implemented checks for each step.
c error-handling memory-management constructor
c error-handling memory-management constructor
edited 39 mins ago
200_success
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited 39 mins ago
200_success
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited 39 mins ago
200_success
130k17156420
edited 39 mins ago
200_success
130k17156420
edited 39 mins ago
200_success
130k17156420
130k17156420
asked 4 hours ago
utkumadenutkumaden
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
asked 4 hours ago
utkumadenutkumaden
62
asked 4 hours ago
utkumadenutkumaden
62
62
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
utkumaden is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not withgotoconstructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.
$endgroup$
– Mast
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct namedpackagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago
|
show 1 more comment
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not withgotoconstructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.
$endgroup$
– Mast
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct namedpackagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not with
goto constructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.$endgroup$
– Mast
4 hours ago
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not with
goto constructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.$endgroup$
– Mast
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct named
packagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.$endgroup$
– utkumaden
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct named
packagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.$endgroup$
– utkumaden
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago
|
show 1 more comment
0
active
oldest
votes
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
utkumaden is a new contributor. Be nice, and check out our Code of Conduct.
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216545%2fconstructor-for-a-packagetarget-struct%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
0
active
oldest
votes
0
active
oldest
votes
active
oldest
votes
active
oldest
votes
utkumaden is a new contributor. Be nice, and check out our Code of Conduct.
draft saved
draft discarded
utkumaden is a new contributor. Be nice, and check out our Code of Conduct.
utkumaden is a new contributor. Be nice, and check out our Code of Conduct.
utkumaden is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f216545%2fconstructor-for-a-packagetarget-struct%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
"However I thought this could be an appropriate case, and would like confirmation or criticism." Yet you've stripped the code out of all it's context so we can't determine whether you're right or not. The code you've provided looks like it should've been used in a wrapper, not with
gotoconstructs. Please provide more code and an explanation of what it's supposed to do so we can see how this snippet is being used. Stack Overflow likes minimal examples, Code Review does absolutely not. Please take a look at our FAQ on asking questions.$endgroup$
– Mast
4 hours ago
$begingroup$
Not all of the functions are written, yet. This function is a constructor for a struct named
packagetarget. The idea is to allocate the struct in memory, and then call the setter methods for each field, which may fail. EDIT: If any step fails, the destructor function is called, once; and the function returns a null pointer.$endgroup$
– utkumaden
4 hours ago
$begingroup$
If not all the functions are written, are you sure it works the way it should? And how can you be sure this is the right way to do it if you haven't completed the rest? It sounds like you're simply too early in the process got get this meaningfully reviewed and your question answered.
$endgroup$
– Mast
4 hours ago
$begingroup$
I have updated the question to include more code.
$endgroup$
– utkumaden
4 hours ago
$begingroup$
What is a constructor in C? I know what a constructor is in C++. Did you mean C++?
$endgroup$
– pacmaninbw
3 hours ago