What could be handled more nicely in this piece of code?

What could be handled more nicely in this piece of code?
#include
#include

struct phonebook_entry {
char *fname;
char *lname;
char *phone;
};

int main(int argc, char **argv)
{
struct phonebook_entry *e = NULL;

e = (struct phonebook_entry *) malloc(sizeof(struct phonebook_entry));

size_t _f, _l, _p;
getline(&e->fname, &_f, stdin);
getline(&e->lname, &_l, stdin);
getline(&e->phone, &_p, stdin);

printf("%s", e->fname);
printf("%s", e->lname);
printf("%s", e->phone);

free(e->fname);
free(e->lname);
free(e->phone);
free(e);

return 0;
}

Other urls found in this thread:

man7.org/linux/man-pages/man3/getline.3.html
twitter.com/SFWRedditImages

use Visual Basic you fucking college student pleb

typdef struct {
}phonebook_entry;

>getline(&e->fname, &_f, stdin);
> getline(&e->lname, &_l, stdin);
> getline(&e->phone, &_p, stdin);
> printf("%s", e->fname);
> printf("%s", e->lname);
> printf("%s", e->phone);
"beautiful"

Write it in Python

class phonebook_entry(object):
fname = ''
lname = ''
phone = ''

if __name__ == "__main__":
e = phonebook()
e.fname, e.lname, phone = input("Please enter nigger over and over: ").split(" ")

Not using C.

Python is so fucking ugly

>declaring size_t variables without initializing them
you're passing random data as your buffer size and writing to an uninitialized pointer inside the struct.
you need to malloc each pointer inside the struct and pass that size as the second parameter in getline()

>not initializing anything
It's like you actually want things like heartbleed to happen.

>malloc
>The malloc() function allocates size bytes and returns a pointer to the allocated memory. The memory is not initialized.
Use calloc instead and set _f, _l, and _p to zero to guarantee that getline will get you a new fresh freeable string

#include

int main(void) {
install_gentoo();
return 0;
}

#include

typedef struct {
char *fname, *lname, *phone;
} phonebook_entry;

int main() {
phonebook_entry e;
scanf("%ms %ms %ms", &e.fname, &e.lname, &e.phone);
printf("%s\n%s\n%s\n", e.fname, e.lname, e.phone);
free(e.fname);
free(e.lname);
free(e.phone);
}

And also check for NULL after calloc and getline, and you can summarise the three consecutive printf's in one call

>Use calloc instead
not really necessary since you will be filling the allocated memory with a null terminated string anyways
>set the read buffer size to zero
then how will he read any data

struct phonebook_entry *e = malloc(sizeof(*e));

man7.org/linux/man-pages/man3/getline.3.html
> If *lineptr is set to NULL and *n is set 0 before the call, then getline() will allocate a buffer for storing the line. This buffer should be freed by the user program even if getline() failed.

>not reading the manpages

>Visual Basic
C'mon man, if you're gonna use a .NET language, at least use C# or F#. VB is trash.

OP here, thanks anons for all your answers.
With the suggestions here I changed my code for error checking and also initialized size holders to 0 so getline now allocates the buffers in the struct.
#include
#include
#include
#include

typedef struct {
char *fname;
char *lname;
char *phone;
} phonebook_entry;

int main(int argc, char **argv)
{
size_t _f = 0, _l = 0, _p = 0;
phonebook_entry *e = NULL;
e = (phonebook_entry *) malloc(sizeof(phonebook_entry));

printf("First name: ");
if(getline(&e->fname, &_f, stdin) == -1) {
fprintf(stderr, "getline_fname: %s\n", strerror(errno));
exit(EXIT_FAILURE);
}

printf("Last name: ");
if(getline(&e->lname, &_l, stdin) == -1) {
fprintf(stderr, "getline_lname: %s\n", strerror(errno));
exit(EXIT_FAILURE);
}

printf("Phone number: ");
if(getline(&e->phone, &_p, stdin) == -1) {
fprintf(stderr, "getline_phone: %s\n", strerror(errno));
exit(EXIT_FAILURE);
}

printf("%s%s%s", e->fname, e->lname, e->phone);

free(e->fname);
free(e->lname);
free(e->phone);
free(e);

return 0;
}

>If *lineptr is set to NULL and *n is set 0 before the call,
then getline() will allocate a buffer for storing the line.
Initialize those string pointers to NULLs

This, use calloc instead of malloc

$ valgrind ./a.out
==13215== Memcheck, a memory error detector
==13215== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==13215== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==13215== Command: ./a.out
==13215==
==13215== Conditional jump or move depends on uninitialised value(s)
==13215== at 0x4E9FC42: getdelim (iogetdelim.c:63)
==13215== by 0x4007EB: main (in /tmp/hellow/a.out)

you want the following:
e = (phonebook_entry *) calloc(1, sizeof(phonebook_entry));

I fixed that by assigning the return of getline to a variable and check it against the condition
thanks user

Why?

Still wrong, you need to initialize the pointers to NULL. Treat the causes, not the symptoms!

goto ;

I tried both malloc and calloc, and booth seem to initialize struct's members to NULL

malloc will generally do that but isn't guaranteed to

That is undocumented, unguaranteed behaviour on the part of your particular implementation of malloc

Thanks for pointing that out user

It's well documented somewhere, just not guaranteed by the standard.

>not guaranteed by the standard
For standard purposes, it's undocumented :^)

Take your meds, grandpa
#include
#include
class Contact {
private:
std::string fName, lName;
int number;

public:
Contact() {}
Contact(std::string fn, std::string ln, int n)
: fName{fn}, lName{ln}, number{n} {}
friend std::ostream& operator> c.number;
return is;
}
};

int main() {
Contact p1, p2, p3, p4, p5;
std::cin >> p1 >> p2 >> p3 >> p4 >> p5;
std::cout

>Using meme arrows in code
:^)

Just like any other python program, it's a half assed, error prone piece of trash that doesn't solve the actual issue

import phone book

typedef struct phonebook_entry {
char *fname;
char *lname;
char *phone;
} phonebook_entry;

int main() {
phonebook_entry e = {};
size_t unused;
getline(&e.fname, &unused, stdin);
getline(&e.lname, &unused, stdin);
getline(&e.phone, &unused, stdin);
printf("%s", e.fname);
printf("%s", e.lname);
printf("%s", e.phone);
}


Used a typedef

Denoted a variable as unused instead of prefixing it with an underscore, which is bad practice except for members that should not be depended on if you provide a public API (which you don't).

Used static initialization to ensure that the entire struct was initialized with zeros

Didn't use calloc/malloc because for a problem this simple, you don't need it. If you were going to store an array of phone entries it might be helpful.

Another difference, If I had been using them, I wouldn't have casted their return values.

Indicated the type of variables in a more readable way

Dropped the implied return

>Dropped the implied return
Nonstandard, noncompliant. You'd fail if I were to correct that.

Damn C++ is fucking disgusting.

Not if he's using C99 or C11.

What said. Also static initialization of structs was introduced in C99 and I'm using it in my program, so there's really no excuse except not knowing the standard.

Huh, I thought it was an excercise on memory management. Still, implied returns are disgusting in C.

Enjoy your
struct phonebook_entry *e = NULL;

e = (struct phonebook_entry *) malloc(sizeof(struct phonebook_entry));

> casting a malloc return value in plain C
How about you actually learn C before trying to criticize it, you dumb pajeet.

Also, the irony here is that without the cast that actually looks like short, simple and clean code, as opposed to the fucking C++ abomination above.

There is no need to learn C anymore.
And if you are talking about "short" programs, C doesn't do a good job here either. Sucks being a C tard, doesn't it?

Also, if you use typedef you can leave out the 'struct'. There's also no reason to initialize e to NULL since malloc returns NULL on failure anyway:

phonebook_entry *e = malloc(sizeof(phonebook_entry));

>pleb
>suggests use VB 6
kys

This is cleaner

I know. It makes C look like a dream.

Brilliant

Idiot. You cannot insert data from scanf into unallocated memory buffer. You would likely get segfault. You are the reason why people think C is shitty.

>casting malloc in C

He's using a POSIX extension: 'm'. It automatically allocates a buffer large enough.

If you're saying this looks like anything remotely good then you're out of your god damn mind, it looks like it was spitted out of an compiler.

>private:
class defaults to private senpai

>using visual bashit
Just kys

I personally use this neat "trick"
>phonebook_entry *e = malloc(sizeof(*e));

It's not good, but it's more sophisticated, unlike the half-arsed C examples ITT

...

...

e = malloc(sizeof(*e));

Everything is in the struct in the original code, so it would have been wiser to put members in public as well. Otherwise I like this C++ code more than C.

>casting malloc
I want sepples niggers OUT

That trick will get you later into trouble when the compiler will refuse to assign void* to a non-void* variable.
Been there and it wasn't fun.

I always used to "typdef struct" but stopped doing that when I started to realize that it creates an anonymous structure. It basically declares two things. And working with anonymous structs makes code more sensitive to undesired effects.
Yes, it has the effect that I need to add the struct keyword more, but I learnt the hard way.

struct {
char *fname;
char *lname;
char *phone;
}
struct phonebook_entry *e;
e = (struct phonebook_entry *) malloc(*e);

forgot the obvious

struct phonebook_entry {
char *fname;
char *lname;
char *phone;
}
struct phonebook_entry *e;
e = (struct phonebook_entry *) malloc(*e);

Don't cast the return type of malloc.

Good code. Could be better but pretty decent.

and...
struct phonebook_entry *e;
e = (struct phonebook_entry *) malloc(*e);

is not the same as

struct phonebook_entry *e = (struct phonebook_entry *) malloc(*e);


For the latter, the compiler will try to assign all variables with its initialising value when entering the block, ONCE. You might think the assignment will take always place on the line you put it but the compiler will certainly think different.