Μοίρασμα buffer σε μικρότερους σε C

[Timonkaipumpa]

Αυτό που σου έγραψε ο migf1. Αυτό που έγραψες ψιλό-χοντρό-μπάζει. 

[defacer]

wtf...

 

Καταρχήν μπορείς πολύ απλά να κάνεις copy 64 byte κάθε φορά όπως λένε και τα παιδιά έτσι

memcpy (buffer_64, buffer_500, 64); // τα πρώτα 64
memcpy (buffer_64, buffer_500 + 64, 64); // τα επόμενα 64
memcpy (buffer_64, buffer_500 + 128, 64); // τα επόμενα 64

Αλλά αυτό δεν υπάρχει περίπτωση να χρειάζεται να το κάνεις απλά και μόνο για να περάσεις μετά τον buffer_64 σε κάποια άλλη συνάρτηση ας την πούμε foo (για να τα στείλεις πχ) για τον πολύ απλό λόγο ότι η foo σίγουρα έχει κάποιο τρόπο να της πεις πόσο μεγάλος είναι ο buffer που της πέρασες. Οπότε απλά της περνάς κατευθείαν το σχετικό τμήμα του buffer_500 και ούτε copy ούτε τίποτα.

[CtrlFreak]

Αυτό είναι το αρχικό κομμάτι που στέλνει μέσω usb

USBInHandle = HIDTxPacket(CUSTOM_DEVICE_HID_EP, (uint8_t*)&ToSendDataBuffer[0],64);

[Timonkaipumpa]

εάν η μορφή της HIDTxPacket είναι:

HIDTxPacket(DeviceHandle, source, amount_to_sent)

Γιατί μετά δεν κάνεις:

ToSendDataBuffer[64], 64

???

 

 

Και μετά:

ToSendDataBuffer[128],64

γενικά:

 

 

ToSendDataBuffer[i*64], 64

Υ.Γ. Εάν είναι για εργασία σε σχολή θα το μετανιώσω που απάντησα...

[migf1]

Δεν έκατσα να το ψειρίσω το loop, οπότε ενδεχομένως να υλοποιείται με πιο απλή λογική, αλλά εφόσον επιβεβαίωσες πως ισχύει αυτό που έγραψε ο defacer (πως δηλαδή η send σου δέχεται το μήκος ως όρισμα) τότε...

 

(πες πως η bufusb_print() είναι η ToSendDataBuffer()... επίσης δεν καλώ την srand() για να σου βγάζει πάντα τα ίδια random bytes, για να κάνεις δοκιμές )...

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <inttypes.h>
#include <ctype.h>

#define BUF_SIZE	500
#define BUF_DATALEN	67    // must be less or equal to BUF_SIZE
#define USB_BUFSIZE	64

/*******************************************//**
 * 
 ***********************************************
 */
void bufusb_print( uint8_t *buf, size_t len )
{
	for (size_t i=0; i < len && i < USB_BUFSIZE; i++) {
		if ( isprint(buf[i]) ) {
			printf( "%0X ", buf[i] );
		}
		else {
			printf( "%s ", ".." );
		}
		fflush( stdout );
	}
	putchar( '\n' );
}


/*******************************************//**
 * Application's entry point.
 ***********************************************
 */
int main( void )
{
	uint8_t buf[BUF_SIZE] = {0};

	// fill buffer with BUF_DATALEN pseudo-random bytes
	for (int i=0; i < BUF_DATALEN; i++) {
		buf[i] = (uint8_t) rand() % 256; 
	}

	int more = BUF_DATALEN % USB_BUFSIZE;
	for (int i=0; i < BUF_DATALEN; ) {
		int j = (i + more) == BUF_DATALEN ? more : USB_BUFSIZE;
		bufusb_print( &buf[i], j );
		i += j;
	}

	return 0;
}

Υ.Γ. Εάν είναι για εργασία σε σχολή θα το μετανιώσω που απάντησα...

Εγώ δηλαδή τι να πω;

[παπι]

https://ideone.com/cABfan

[gon1332]

Όπα τι σημειογραφία? Αν κάνω κάτι τέτοιο?

 

memcpy( 64bytebuffer, *buffer[65], 64 );

 

Ό,τι σου είπε ο migf1 και οι υπόλοιποι. Όταν έλεγα σημειογραφία εννούσα ότι έχει σχέση με pointers (πράξεις κ.α.). Δηλαδή buffer+64.

 

Απλά ανέφερα ότι οι πίνακες δεν είναι pointers γιατί αυτό ισχύει.

 

 

Σκέψου το παρακάτω κώδικα:

char array[9] = "happiness";
...
c = array[i];

O compiler έχει αποθηκευμένη μέσα του, τη διεύθυνση του πίνακα array, έστω 5450.

Κατά τη διάρκεια εκτέλεσης (run-time) του παραπάνω κώδικα συμβαίνουν δύο βήματα:

1. πάρε την τιμή του i και πρόσθεσέ τη στη διεύθυνση του array, δηλαδή (5450 + i)
2. πάρε τα δεδομένα που βρίσκονται στη διεύθυνση (5450 + i)

Βλέπε εικόνα array.png

 

 

Έστω τώρα ότι έχουμε αυτόν τον κώδικα:

char *pointer = "sorrow";
...
c = pointer[i]; 

O compiler έχει αποθηκευμένη μέσα του, τη διεύθυνση του pointer, έστω 5450.

Κατά το run-time, ο παραπάνω κώδικας εκτελείται σε τρία βήματα:

1. πάρε τα δεδομένα της διεύθυνσης 5450, ας πούμε '9880'.
2. πάρε την τιμή του i και πρόσθεσέ τη στο 9880.
3. πάρε τα δεδομένα της διεύθυνσης (9880 + i)

Βλέπε εικόνα pointer.png

 

 

 

Συγγνώμη αν σε μπέρδεψα.

[CtrlFreak]

Για πτυχιακή είναι και η απάντησή σας είναι μια σταγόνα στον ωκεανό. Αύριο θα κοιτάξω να δω πως ορίζεται η συνάρτηση που στέλνει τα δεδομένα και συνεχίζω. Ευχαριστώ! 

εάν η μορφή της HIDTxPacket είναι:

HIDTxPacket(DeviceHandle, source, amount_to_sent)

Γιατί μετά δεν κάνεις:

ToSendDataBuffer[64], 64

???

 

 

Και μετά:

ToSendDataBuffer[128],64

γενικά:

ToSendDataBuffer[i*64], 64

Υ.Γ. Εάν είναι για εργασία σε σχολή θα το μετανιώσω που απάντησα...

 

Κάτι τέτοιο είχα στο μυαλό μου όταν διάβασα το μνμ του defacer

[migf1]

... 

Κάτι τέτοιο είχα στο μυαλό μου όταν διάβασα το μνμ του defacer

Πρόσεξε όμως γιατί αυτό από μόνο του δεν σε εξασφαλίζει στην περίπτωση που το μεγάλο buffer δεν είναι φουλ γεμισμένο. Όπως σου έγραψα και στην 1η σελίδα, σε αυτή την περίπτωση πρέπει να περάσεις στην τελευταία κλήση της ToSendDataBuffer() ως 2ο όρισμα το BUF_DATALEN % USB_BUFSIZE. Διότι εφόσον μιλάμε για binary data το πιθανότερο είναι τα μηδενικά bytes να λογίζονται ως έγκυρα data.

 

Αν λοιπόν δεν υπάρχει κάποιο sentinel byte (είτε το μηδενικό είτε κάποιο άλλο) ώστε να μπορεί η ToSendDataBuffer() να καταλάβει που σταματάνε τα έγκυρα data στο 64άρι buffer που της περνάς, τότε είναι επιβεβλημένο να της περνάς στην τελευταία κλήση της το μήκος των έγκυρων data (αντί δηλαδή για 64). Περίπου το ίδιο ισχύει κι αν το μήκος του αρχικού σου buffer δεν διαιρείται με το 64 (και σε αυτό που μας έδωσες ως δεδομένο, το 500 ΔΕΝ διαιρείται ακριβώς με το 64).

 

Τη λύση στην έχουμε δώσει στο πιάτο και εγώ και ο παπι. Του παπι είναι πιο clean υλοποίηση (και πιο γρήγορη). Αντί να έχεις ένα έξτρα if-else μέσα σε κάθε iteration του loop (όπως το έχω εγώ), κάνεις 1 loop μέχρι BUF_DATALEN / USB_BUFSIZE και κατόπιν περνάς και τα περισσευούμενα BUF_DATALEN % USB_BUFSIZE bytes.

[defacer]

Δεν υπάρχει λόγος να κάνει τέτοια πράγματα. Much cleaner να γίνουν όλα με ένα απλό while (bytesToSend):

uint8_t* data = bigBuffer /* ή κάτι τέτοιο */
int bytesToSend = 500;

while (bytesToSend) {
    int batchSize = std::min(bytesToSend, 64);
    send(data, batchSize);
    bytesToSend -= batchSize;
    data += batchSize; // ευγενική προσφορά του geomagas, βλ. παρακάτω
}

Το να αντικαταστήσει κανείς ένα απλό min() με έξτρα πράξεις και έξτρα μία επανάληψη στο τέλος είναι το είδος του κώδικα που γράφει κανείς όταν μόλις μαθαίνει. Επιπλέον, αν τύχει ο αριθμός που θέλεις να στείλεις να είναι ακέραιο πολλαπλάσιο του π.χ. 64 τότε έτσι που το έχετε θα κάνει ένα ακόμα send με μήκος μηδέν, το οποίο μπορεί πολύ άνετα να μην επιτρέπεται (δεν έχουμε ιδέα τι κάνει αυτή η send).

 

Simple is good.

Επεξ/σία 26 Μαΐου 2014 από defacer

[παπι]

Οντως, much cleaner.

[migf1]

Αφενός στη C δεν υπάρχει min, οπότε πρέπει να τη γράψει, αφετέρου επιβαρύνει τη λούπα με αχρείαστο if-else, και... αφ-τρίτου αυτό δεν διαφέρει από το δικό μου κώδικα (που είναι κακός).

 

Ο ιδανικός κώδικας για κάποιον που δεν μαθαίνει τώρα, ο κώδικας δηλαδή που συνδυάζει readability με efficiency είναι αυτός που έδωσε ο παπι. Ακόμα και χωρίς εσωτερικό sanity check για τα ορίσματά της, η send() είναι 99.9999999999% σίγουρο πως έχει εσωτερική λούπα for (i=0; i < len; ) η οποία αυτόματα δεν εκτελείται καν με len < 1.

[defacer]

Επειδή δε θέλω να είμαι μονάχα αρνητικός θα συμφωνήσω σε κάτι: ότι ο δικός σου κώδικας είναι κακός.

 

 

 

Just kidding. Το ξέρω πως ό,τι περάσει από τα χέρια σου γίνεται η τελειότερη λύση που συνέλαβε ποτέ ανθρώπινος νους.

 

 

 

Θα έλεγα ότι μπορούμε να πάμε στο http://codereview.stackexchange.com και να ζητήσουμε γνώμες για το ποιά μορφή είναι προτιμότερη, αλλά ξέρω ότι δεν έχεις τις γνώμες των άλλων σε μεγάλη υπόληψη οπότε τζάμπα κόπος.

 

Τέλος πάντων για το υπόλοιπο κοινό, προσωπικά θεωρώ τη λύση με το while (ακριβώς έτσι) καλύτερη γιατί πιο άμεσα κατανοητή δε γίνεται:

while (bytesToSend) {
   // χμμμ... τι να κάνει άραγε αυτό το loop?
}

YMMV.

[migf1]

Επειδή δε θέλω να είμαι μονάχα αρνητικός θα συμφωνήσω σε κάτι: ότι ο δικός σου κώδικας είναι κακός.

 

 

 

Just kidding. Το ξέρω πως ό,τι περάσει από τα χέρια σου γίνεται η τελειότερη λύση που συνέλαβε ποτέ ανθρώπινος νους.

 

 

Κι εφόσον επί της ουσίας είναι ίδιος με τον δικό σου, συνεπάγεται πως και ο δικός σου είναι κακός.

 

Θα έλεγα ότι μπορούμε να πάμε στο http://codereview.stackexchange.com και να ζητήσουμε γνώμες για το ποιά μορφή είναι προτιμότερη, αλλά ξέρω ότι δεν έχεις τις γνώμες των άλλων σε μεγάλη υπόληψη οπότε τζάμπα κόπος.

 

Τέλος πάντων για το υπόλοιπο κοινό, προσωπικά θεωρώ τη λύση με το while (ακριβώς έτσι) καλύτερη γιατί πιο άμεσα κατανοητή δε γίνεται:

while (bytesToSend) {
   // χμμμ... τι να κάνει άραγε αυτό το loop?
}

YMMV.

 

Αν θεωρείς πως το συγκεκριμένο είναι αμφιλεγόμενο, feel free να το στείλεις όπου θέλεις για code review. Ελπίζω μονάχα να το στείλεις ακριβώς έτσι όπως το έχεις γράψει για C reviewing... και ως αντιπαράθεση να δώσεις τον κώδικα του παπι χωρίς τις σάλτσες που έχει βάλει...

 

void send_wrapper( uint8_t *data )
{
    assert( data );

    int frags = len / 64;
    for (int i=0; i < frags; i++ ) {
        send( data, 64 )
        data += 64;
    }
    send( data, len % 64 );
}

EDIT-1:

A, και btw, ο παραπάνω απλοποιημένος κώδικας του πάπι ή πρέπει να μπει σε συνάρτηση (όπως ο αρχικός του) ή να δουλέψει με indices αντί για += (όπως ο αρχικός ο δικός μου), ή τουλάχιστον να κάνει save την αρχή του buffer, αλλιώς πάει το 'χασες μετά την πρώτη φουρνιά αποστολών.

 

EDIT 2:

Εφάρμοσα το edit-1 στον κώδικα (το έκανα συνάρτηση δηλαδή).

[defacer]

Κι εφόσον επί της ουσίας είναι ίδιος με τον δικό σου, συνεπάγεται πως και ο δικός σου είναι κακός.

 

Εφόσον επί της ουσίας το να κάνεις π.x. hardcode κάτι είναι ίδιο με το να το περνάς σαν παράμετρο, οι δυο πρακτικές είναι το ίδιο καλές. Κι εφόσον επί της ουσίας το copy paste είναι το ίδιο με το να κάνεις function, το ίδιο καλό είναι και το copy paste.

 

Παρόλο που δεν έχω καμία αντίρρηση στο να χαρακτηρίσεις τον κώδικά μου κακό (δεν τον έγραψα δα και για να πάρει βραβείο), δεν πάει έτσι.